From f2ec77ace4978bd0864b893cff6a0f3c9072fdba Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 18:46:21 -0600 Subject: [PATCH 01/16] fix(release): close 4 real release-path bugs found by pr-review ensemble MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran pr-review (codex-cli + copilot-cli ensemble) retroactively against the just-merged direct-publish flow (PR #4). Both reviewers caught the same two release-path bugs; codex caught a third about breaking-change parsing; copilot caught a least-privilege gap. All four are real and fixable in one focused PR. 1. [medium] Single-package version gate skipped manual changesets. The workflow compared only `packages/eslint-config/package.json` before/after `changeset:version`. A valid manual changeset bumping only `@happyvertical/prettier-config` or `@happyvertical/tsconfig-base` would update those package files, pass through `changeset:version`, and then hit the eslint-only gate which sees no change → exit 0 without commit/publish. The manual release silently vanishes. Fixed by gating on the working tree itself (`git diff --quiet && git diff --cached --quiet`) after `changeset:version`. Catches every case: auto-generated bumps to all packages, manual changesets that touch any subset, and no-op runs where nothing was releasable. 2. [medium] Publish ran before push, allowing registry/git drift. Original order: commit → publish → push. If main advanced mid-run (concurrency only serializes release runs, not human pushes), publish would succeed and the push would fail non-fast-forward. Result: registry has new versions, main doesn't have the bump commit, next run tries to republish. Fixed by reordering: commit → fetch origin/main → verify it hasn't moved past our parent → push commit → publish → push tags. If origin/main advanced, we abort BEFORE publishing with a clear error message; the next push to main re-runs and picks up the new state. The narrow window where publish runs after push but before tags-push can still produce missing tags on a partial publish failure, but that's recoverable (rerun `changeset:publish`; published versions are idempotent). 3. [medium] Breaking-change footers were ignored. auto-changeset.ts read commit subjects only via `--pretty=format:%H|||%s` and matched `BREAKING CHANGE` against the subject. Standard Conventional Commits put breaking markers in a body footer: feat: change config shape BREAKING CHANGE: consumers must update their extends path That commit was being classified as patch — shipping a breaking change under a patch version. Fixed by including `%b` in the log format (with sentinels for safe parsing across newlines and pipes in bodies), passing the body through to `isBreaking()`, and matching both `BREAKING CHANGE` and `BREAKING-CHANGE` in subject OR body. 4. [low] `id-token: write` permission was declared but no step uses OIDC. GitHub Packages publishing doesn't need it; we don't mint attestations. Removed per least-privilege. Smoke test: `pnpm changeset:auto` defers correctly to existing manual changeset infrastructure. --- .github/workflows/release.yml | 65 ++++++++++++++++++++++------------- scripts/auto-changeset.ts | 54 ++++++++++++++++++++--------- 2 files changed, 78 insertions(+), 41 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 891261d..b517d71 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,7 +12,6 @@ concurrency: permissions: contents: write packages: write # publish to GitHub Packages - id-token: write # attestations jobs: release: @@ -65,7 +64,7 @@ jobs: git config user.name "have-release-bot" git config user.email "have-release[bot]@users.noreply.github.com" - - name: Auto-changeset, version, commit, publish + - name: Auto-changeset, version, commit, push, then publish env: GITHUB_TOKEN: ${{ steps.token.outputs.token }} NODE_AUTH_TOKEN: ${{ steps.token.outputs.token }} @@ -73,13 +72,6 @@ jobs: run: | set -euo pipefail - # Use eslint-config as the canonical version source (all three - # packages are version-locked by auto-changeset). - BEFORE=$(node -p "require('./packages/eslint-config/package.json').version") - echo "::group::Before" - echo "Version before: $BEFORE" - echo "::endgroup::" - # 1. Generate a changeset from conventional commits since the # last release (or defer if a manual changeset is present). pnpm changeset:auto @@ -88,29 +80,54 @@ jobs: # CHANGELOG.md entries + deletes consumed .changeset/*.md. pnpm changeset:version - AFTER=$(node -p "require('./packages/eslint-config/package.json').version") - echo "::group::After" - echo "Version after: $AFTER" - echo "::endgroup::" - - if [ "$BEFORE" = "$AFTER" ]; then - echo "No version change — nothing to publish." + # 3. Gate on whether anything actually changed. Using the + # working tree (not a single package's version) catches + # every case: auto-generated bumps to all packages, + # manual changesets that bump only one package, no-op + # runs where nothing was releasable. Avoids the + # single-package gate trap (manual changeset for only + # prettier-config would have skipped publish under the + # old eslint-only check). + if git diff --quiet && git diff --cached --quiet; then + echo "No releasable changes after changeset:version — nothing to publish." exit 0 fi - echo "Bumping $BEFORE → $AFTER" + # Use eslint-config as a representative version for logging. + # All publishable packages bump together under auto-changeset + # (manual changesets may bump a subset). + AFTER=$(node -p "require('./packages/eslint-config/package.json').version") + echo "Releasing version $AFTER" - # 3. Commit the version bumps + changelog entries. The chore + # 4. Commit the version bumps + changelog entries. The chore # prefix matches what the job-level `if:` guard filters. git add -A git commit -m "chore(release): v$AFTER" - # 4. Publish to GitHub Packages. `changeset publish` also + # 5. Verify main hasn't moved out from under us, then push + # the commit BEFORE publishing. Order matters: if publish + # runs first and then the push fails (because main + # advanced mid-job), we get registry/git drift — registry + # has the new versions, main doesn't have the bump + # commit, and the next run tries to republish. + # + # Push order: commit first, then publish, then tags. + # `changeset:publish` creates per-package tags locally; + # we push them afterwards. + git fetch origin main + if [ "$(git rev-parse origin/main)" != "$(git rev-parse HEAD~1)" ]; then + echo "::error::origin/main advanced during this run. Aborting before publish to avoid registry/git drift. The next push to main will re-run and pick up the new state." + exit 1 + fi + git push origin main + + # 6. Publish to GitHub Packages. changeset:publish also # creates per-package git tags (e.g. - # `@happyvertical/eslint-config@0.2.0`). + # `@happyvertical/eslint-config@0.2.0`). If publish + # partially fails, the commit is already on main so + # state is at least consistent; manual rerun of publish + # can recover (idempotent for already-published versions). pnpm changeset:publish - # 5. Push the release commit AND the tags created by publish. - # --follow-tags ensures any annotated/lightweight tags - # pointing at HEAD go with the push. - git push --follow-tags origin main + # 7. Push the per-package tags created by changeset:publish. + git push --tags origin main diff --git a/scripts/auto-changeset.ts b/scripts/auto-changeset.ts index 8a541f8..ffe5468 100644 --- a/scripts/auto-changeset.ts +++ b/scripts/auto-changeset.ts @@ -12,10 +12,20 @@ * see in the changelog. * * Bump rules for 0.x.x releases (everything stays pre-1.0): - * - Any commit with `!` after type/scope, or `BREAKING CHANGE` in the - * subject → minor bump + * - Any commit with `!` after type/scope, or `BREAKING CHANGE` / + * `BREAKING-CHANGE` in the subject OR body → minor bump * - All other commits (including `docs:`, `chore:`, `ci:`) → patch bump * + * Body inspection matters: the Conventional Commits spec puts breaking + * markers in a footer, e.g. + * + * feat: change config shape + * + * BREAKING CHANGE: consumers must update their extends path + * + * Subject-only inspection would classify this as a patch and ship a + * breaking change under a patch version. We scan the body too. + * * `chore(release):` commits are filtered out — they're the workflow's * own version-bump commits and including them would re-bump on the * next run. @@ -35,8 +45,14 @@ const PACKAGES = [ interface Commit { hash: string; subject: string; + body: string; } +// Sentinel between commit fields and between commits, picked to be +// vanishingly unlikely in real commit text. +const FIELD_SEP = '\x00FIELD\x00'; +const COMMIT_SEP = '\x00COMMIT\x00'; + /** * Run a git subcommand with args passed positionally. Uses execFileSync * so no shell parsing occurs. Returns trimmed stdout, or empty string on @@ -74,17 +90,19 @@ function getLastReleaseTag(): string | null { function getCommitsSinceLastRelease(): Commit[] { const lastTag = getLastReleaseTag(); const range = lastTag ? `${lastTag}..HEAD` : 'HEAD'; - const log = git( - 'log', - range, - '--pretty=format:%H|||%s', - '--no-merges', - ); + // Include body via %b so we can detect BREAKING CHANGE: footers. + // Use unlikely sentinels because bodies contain newlines and pipes. + const format = `%H${FIELD_SEP}%s${FIELD_SEP}%b${COMMIT_SEP}`; + const log = git('log', range, `--pretty=format:${format}`, '--no-merges'); if (!log) return []; - return log.split('\n').map((line) => { - const [hash, ...subjectParts] = line.split('|||'); - return { hash: hash ?? '', subject: subjectParts.join('|||') }; - }); + return log + .split(COMMIT_SEP) + .map((entry) => entry.trim()) + .filter(Boolean) + .map((entry) => { + const [hash = '', subject = '', body = ''] = entry.split(FIELD_SEP); + return { hash, subject, body }; + }); } function hasManualChangesets(): boolean { @@ -101,11 +119,13 @@ function hasManualChangesets(): boolean { function isBreaking(commit: Commit): boolean { // `type!: ...` or `type(scope)!: ...` if (/^[a-z]+(\([^)]+\))?!:/.test(commit.subject)) return true; - // `BREAKING CHANGE:` in subject. Body inspection would be more - // complete but `git log --format=%s` only gives subjects; relying on - // the convention of putting BREAKING CHANGE in the subject when - // authors mean it. - if (/\bBREAKING CHANGE\b/.test(commit.subject)) return true; + // `BREAKING CHANGE:` or `BREAKING-CHANGE:` in the subject OR body. + // The Conventional Commits spec places these in a footer, so the + // body check is the canonical case; subject check catches the (less + // common) inline form. + const breakingPattern = /\bBREAKING[- ]CHANGE\b/; + if (breakingPattern.test(commit.subject)) return true; + if (breakingPattern.test(commit.body)) return true; return false; } From 10f879f248ab4fe5b205727f515290f98dbe9cea Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 20:34:29 -0600 Subject: [PATCH 02/16] =?UTF-8?q?fix(release):=20round-2=20=E2=80=94=20NUL?= =?UTF-8?q?=20byte=20argv=20fix=20+=20workflow=5Fdispatch=20recovery?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 fix in f2ec77a introduced two bugs that the round-2 ensemble caught: 1. [HIGH] `\x00FIELD\x00` / `\x00COMMIT\x00` sentinels in argv Node's child_process refuses NUL bytes in argv with a TypeError. The try/catch in `git()` swallowed it as "no output", so getCommitsSinceLastRelease() always returned [], real.length === 0, and auto-changeset early-returned every run — silently breaking the whole release pipeline. Fix: use git's pretty-format escapes `%x1f` / `%x1e` which emit ASCII 31 / 30 in the OUTPUT, not in argv. Neither byte appears in commit text, and they don't trip Node's argv validation. Also tightened `git()` to distinguish git's non-zero exits (expected for `describe --tags` with no tags) from Node-level errors (our bugs) — Node-level errors now log to stderr so the next such typo doesn't silently skip releases. 2. [medium] No recovery path for partial publish failure If `pnpm changeset:publish` fails after the version commit was already pushed, the version bump is on main but packages aren't published. Re-running via push is skipped by the chore(release) guard, and the main-advanced check would abort anyway. Fix: add workflow_dispatch trigger with mode=publish-only that skips auto-changeset + version + commit + push and runs only changeset publish against current main. The job-level if: no longer skips workflow_dispatch invocations. --- .github/workflows/release.yml | 58 ++++++++++++++++++++++++++++++----- scripts/auto-changeset.ts | 46 +++++++++++++++++++-------- 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b517d71..b117bdb 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -4,6 +4,23 @@ on: push: branches: - main + # Recovery path for partial publish failures. If `pnpm + # changeset:publish` fails after the version commit was already + # pushed (network glitch, registry outage, transient auth), the + # version bump is on main but packages aren't published. Re-running + # this workflow via the push event would either be skipped by the + # chore(release) guard or hit the main-advanced check. Operators + # can instead trigger `workflow_dispatch` with mode=publish-only to + # check out current main and run just `changeset publish`. + workflow_dispatch: + inputs: + mode: + description: 'release flow mode' + type: choice + options: + - auto # normal: auto-changeset → version → commit → push → publish + - publish-only # recovery: skip auto-changeset and version; just publish from current main + default: auto concurrency: group: release-${{ github.ref }} @@ -18,11 +35,11 @@ jobs: name: Direct-publish runs-on: ubuntu-latest timeout-minutes: 15 - # Skip the workflow's own chore(release) commits. Without this guard, - # every release commit would trigger another (no-op) run. The - # auto-changeset script would catch it too, but skipping at job level - # saves the runner. - if: ${{ !startsWith(github.event.head_commit.message, 'chore(release):') }} + # Skip the workflow's own chore(release) push commits during the + # `push` event. Without this guard, every release commit would + # trigger another (no-op) run. workflow_dispatch invocations are + # never skipped — that's the recovery path. + if: ${{ github.event_name == 'workflow_dispatch' || !startsWith(github.event.head_commit.message, 'chore(release):') }} steps: # Mint a token from the HAVE_RELEASE GitHub App (org-wide secret # pair). Used as GITHUB_TOKEN (for the chore(release) push back to @@ -65,6 +82,7 @@ jobs: git config user.email "have-release[bot]@users.noreply.github.com" - name: Auto-changeset, version, commit, push, then publish + if: ${{ github.event_name != 'workflow_dispatch' || inputs.mode == 'auto' }} env: GITHUB_TOKEN: ${{ steps.token.outputs.token }} NODE_AUTH_TOKEN: ${{ steps.token.outputs.token }} @@ -125,9 +143,35 @@ jobs: # creates per-package git tags (e.g. # `@happyvertical/eslint-config@0.2.0`). If publish # partially fails, the commit is already on main so - # state is at least consistent; manual rerun of publish - # can recover (idempotent for already-published versions). + # state is at least consistent. Recovery: re-trigger this + # workflow with workflow_dispatch + mode=publish-only. + # Versions already published are idempotent under + # `changeset publish`. pnpm changeset:publish # 7. Push the per-package tags created by changeset:publish. git push --tags origin main + + - name: Publish only (recovery from partial publish failure) + if: ${{ github.event_name == 'workflow_dispatch' && inputs.mode == 'publish-only' }} + env: + GITHUB_TOKEN: ${{ steps.token.outputs.token }} + NODE_AUTH_TOKEN: ${{ steps.token.outputs.token }} + NPM_TOKEN: ${{ steps.token.outputs.token }} + run: | + set -euo pipefail + + # Recovery path: a previous auto run committed + pushed the + # version bump but `changeset publish` failed before + # completing (network glitch, registry outage). We're now + # on current main, which contains the unpublished version + # bumps. Run publish only — no auto-changeset, no + # changeset:version (those already happened on the failed + # run). `changeset publish` is idempotent for versions + # already on the registry; it only attempts unpublished + # versions. + echo "Recovery mode: running changeset:publish against current main." + pnpm changeset:publish + + # Push any tags created by the publish. + git push --tags origin main diff --git a/scripts/auto-changeset.ts b/scripts/auto-changeset.ts index ffe5468..b66075b 100644 --- a/scripts/auto-changeset.ts +++ b/scripts/auto-changeset.ts @@ -48,15 +48,27 @@ interface Commit { body: string; } -// Sentinel between commit fields and between commits, picked to be -// vanishingly unlikely in real commit text. -const FIELD_SEP = '\x00FIELD\x00'; -const COMMIT_SEP = '\x00COMMIT\x00'; +// Field/record separators for parsing `git log` output. We need bytes +// that won't appear in commit subjects or bodies and that don't break +// Node's child_process argv handling. Git's `%x` pretty-format +// escape emits the byte in the OUTPUT without requiring us to put it +// in the argv (which would fail — Node rejects NUL bytes in args, and +// even non-NUL control bytes in argv are awkward). We use ASCII 31 +// (Unit Separator) between fields and ASCII 30 (Record Separator) +// between commits — neither appears in real commit text. +const FIELD_SEP_OUT = '\x1f'; +const COMMIT_SEP_OUT = '\x1e'; +const GIT_PRETTY_FORMAT = `%H%x1f%s%x1f%b%x1e`; /** * Run a git subcommand with args passed positionally. Uses execFileSync - * so no shell parsing occurs. Returns trimmed stdout, or empty string on - * non-zero exit (e.g. `git describe` with no tags yet). + * so no shell parsing occurs. Returns trimmed stdout, or empty string + * when git itself exits non-zero (e.g. `git describe` with no tags). + * + * Surfaces non-git errors (invalid argv, missing binary, etc.) by + * logging to stderr and returning empty — we don't want a typo in our + * own format string to look like "no commits" and silently skip a + * release. Caller should treat empty as "no output" but log if needed. */ function git(...args: string[]): string { try { @@ -64,7 +76,14 @@ function git(...args: string[]): string { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], }).trim(); - } catch { + } catch (err) { + // Git returning non-zero is expected for some calls (describe with + // no tags). But Node-level errors (NUL in args, binary not found, + // etc.) indicate our bug, not git's — surface them. + const e = err as { status?: number | null; code?: string; message?: string }; + if (e.status === undefined || e.status === null) { + console.error(`auto-changeset: git ${args.join(' ')} failed: ${e.code ?? e.message ?? err}`); + } return ''; } } @@ -90,17 +109,18 @@ function getLastReleaseTag(): string | null { function getCommitsSinceLastRelease(): Commit[] { const lastTag = getLastReleaseTag(); const range = lastTag ? `${lastTag}..HEAD` : 'HEAD'; - // Include body via %b so we can detect BREAKING CHANGE: footers. - // Use unlikely sentinels because bodies contain newlines and pipes. - const format = `%H${FIELD_SEP}%s${FIELD_SEP}%b${COMMIT_SEP}`; - const log = git('log', range, `--pretty=format:${format}`, '--no-merges'); + // %x1f / %x1e are git pretty-format escapes that emit ASCII 31 / 30 + // in the OUTPUT. We don't put those bytes in argv (Node rejects NUL + // bytes there, and embedding raw control bytes is brittle); git + // expands them when generating the log. + const log = git('log', range, `--pretty=format:${GIT_PRETTY_FORMAT}`, '--no-merges'); if (!log) return []; return log - .split(COMMIT_SEP) + .split(COMMIT_SEP_OUT) .map((entry) => entry.trim()) .filter(Boolean) .map((entry) => { - const [hash = '', subject = '', body = ''] = entry.split(FIELD_SEP); + const [hash = '', subject = '', body = ''] = entry.split(FIELD_SEP_OUT); return { hash, subject, body }; }); } From decfb821be021ade2e6489a42f40d962c83d6c9d Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 20:40:24 -0600 Subject: [PATCH 03/16] =?UTF-8?q?fix(release):=20round-3=20=E2=80=94=20rec?= =?UTF-8?q?overy=20target=20guard=20+=20tag=20repair=20+=20tag-only=20push?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 ensemble found three real release-correctness hazards in the publish-only recovery path added in 10f879f: 1. [HIGH] Recovery could publish the wrong source tree `pnpm changeset:publish` runs against whatever ref the workflow_dispatch checked out. If main has advanced beyond the failed release commit `R` (any commit `N` landed in the interval), recovery publishes R's package.json versions with N's source code — the registry artifact for that version silently contains post-version-commit changes. Fix: guard the recovery step. Refuse to run unless HEAD is a `chore(release):` commit AND HEAD === origin/main. Recovery is only safe immediately after a failed publish, before any new commits land. If main has advanced, operator must resolve manually (revert the new commits, or hand-publish from a clean checkout of `R`). 2. [medium] Recovery doesn't recreate lost local tags In a partial failure, the first run can publish + tag some packages locally before erroring out. Local tags die with the runner. On recovery, `changeset publish` skips already-published versions (correctly idempotent), so the tags are never recreated by publish alone, and `git push --tags` pushes nothing for them. The next auto run's tag-based release boundary then scans too far back and double-includes old commits. Fix: run `pnpm changeset tag` after publish in the recovery step. That reads package.json versions and creates any missing tags regardless of publish outcome. 3. [medium] Tag-push step also pushed the main branch ref `git push --tags origin main` pushes BOTH tags and the main branch. In the auto path, main was already pushed at step 5 before publish ran. If another commit lands between that step-5 push and the step-7 tag push, the redundant main push fails with non-fast-forward — AFTER packages have already published to the registry, leaving registry/git drift. Fix: `git push origin --tags` in both paths. Recovery never needs to advance main; auto already pushed it explicitly. --- .github/workflows/release.yml | 54 ++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b117bdb..c93f8e9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -150,7 +150,13 @@ jobs: pnpm changeset:publish # 7. Push the per-package tags created by changeset:publish. - git push --tags origin main + # Push tags only — main was already pushed at step 5 before + # publish ran. Pushing `main` again here would fail with a + # non-fast-forward rejection if any other commit landed in + # the interval, AFTER packages have already been published + # to the registry. That'd leave registry/git drift the + # publish-only recovery path is meant to avoid. + git push origin --tags - name: Publish only (recovery from partial publish failure) if: ${{ github.event_name == 'workflow_dispatch' && inputs.mode == 'publish-only' }} @@ -170,8 +176,48 @@ jobs: # run). `changeset publish` is idempotent for versions # already on the registry; it only attempts unpublished # versions. - echo "Recovery mode: running changeset:publish against current main." + + # Guard: refuse to publish unless HEAD is the failed release's + # `chore(release):` commit AND nothing new has landed on + # origin/main. Without this, recovery could publish stale + # version numbers from `R` (the failed release commit) with + # the source tree of any later commit `N` that landed in the + # meantime — the registry artifact would silently contain + # post-version-commit code. Recovery is only safe immediately + # after a failed publish, before any new commits land. + git fetch origin main + HEAD_MSG=$(git log -1 --pretty=%s HEAD) + if ! printf '%s' "$HEAD_MSG" | grep -q '^chore(release):'; then + echo "::error::Recovery requires HEAD to be a chore(release): commit. HEAD subject is: $HEAD_MSG" + echo "::error::If the failed release commit is no longer at HEAD, resolve manually — do not use this recovery path." + exit 1 + fi + if [ "$(git rev-parse HEAD)" != "$(git rev-parse origin/main)" ]; then + echo "::error::origin/main has advanced past HEAD. Recovery would publish the wrong tree." + echo "::error::Resolve manually (e.g. revert the new commits, or hand-publish from a clean checkout of the failed release commit)." + exit 1 + fi + + echo "Recovery mode: running changeset:publish against current main ($(git rev-parse --short HEAD))." pnpm changeset:publish - # Push any tags created by the publish. - git push --tags origin main + # Repair tags for packages whose versions are already + # published but missing local tags. In a partial failure, the + # first run can succeed publishing + locally tagging some + # packages before erroring out — those local tags die with + # the runner. On recovery, `changeset publish` skips + # already-published versions (correctly idempotent), so the + # tags are never recreated by publish alone. `changeset tag` + # reads package.json versions and creates any missing tags; + # without it, `git push --tags` would push nothing for those + # packages and the next auto run's tag-based release boundary + # would scan too far back and double-include old commits. + pnpm changeset tag + + # Push tags only — recovery never touches the main branch + # ref. The release commit was already pushed by the failed + # auto run; pushing main again here would only succeed if + # main hadn't advanced (which our guard above already + # ensures), so it'd be redundant. Explicit tag-only push + # keeps the intent clear. + git push origin --tags From 05113478ac6dfb5ff88ee9334285c82696781cb1 Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 20:45:34 -0600 Subject: [PATCH 04/16] =?UTF-8?q?fix(release):=20round-4=20=E2=80=94=20exp?= =?UTF-8?q?licit=20fetch=20refspec=20+=20escape=20commit=20subject?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-4 ensemble caught two more medium-severity correctness hazards in the workflow: 1. [medium] `git fetch origin main` may not update `refs/remotes/origin/main` With a source-only refspec (`main` instead of `main:main`), git's behaviour depends on the remote's configured fetch refspec, shallow state, and partial-clone setup. Under some configurations it only updates FETCH_HEAD, leaving `refs/remotes/origin/main` stale. The subsequent `git rev-parse origin/main` then compares against a stale value — the guard passes even though main has actually advanced, and the publish runs against the wrong tree. Fix: explicit destination refspec `+refs/heads/main:refs/remotes/origin/main` in both auto and recovery paths. The `+` forces non-fast-forward updates so the remote-tracking branch always reflects the remote's current state. 2. [medium] Commit subject embedded in `::error::` workflow command unescaped The recovery guard's failure message interpolates `$HEAD_MSG` (commit subject — user-controlled) directly into a workflow command. GitHub workflow commands require `%`, CR, LF to be escaped (`%` → `%25`, CR → `%0D`, LF → `%0A`) before interpolating user-controlled content; without escaping, a crafted commit subject can corrupt the command payload or inject additional workflow commands into the log parser. Fix: inline bash parameter expansion to escape the three bytes before embedding. Verified the escape produces correct output for subjects containing all three characters. (The non-error `echo` in the success path interpolates only the short SHA from `git rev-parse --short HEAD`, which is hex-only and can't contain `%`/CR/LF — no escaping needed there.) --- .github/workflows/release.yml | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c93f8e9..d63e951 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -132,7 +132,12 @@ jobs: # Push order: commit first, then publish, then tags. # `changeset:publish` creates per-package tags locally; # we push them afterwards. - git fetch origin main + # Explicit destination refspec — `git fetch origin main` with + # a source-only refspec is config-dependent: under some setups + # (custom refspecs, partial/shallow clones) it only updates + # FETCH_HEAD, not `refs/remotes/origin/main`. The `+` forces + # the remote-tracking branch to update non-fast-forward too. + git fetch origin +refs/heads/main:refs/remotes/origin/main if [ "$(git rev-parse origin/main)" != "$(git rev-parse HEAD~1)" ]; then echo "::error::origin/main advanced during this run. Aborting before publish to avoid registry/git drift. The next push to main will re-run and pick up the new state." exit 1 @@ -185,10 +190,20 @@ jobs: # meantime — the registry artifact would silently contain # post-version-commit code. Recovery is only safe immediately # after a failed publish, before any new commits land. - git fetch origin main + # Explicit destination refspec — see auto path above for why + # `git fetch origin main` alone is not reliable for updating + # `refs/remotes/origin/main` under all configurations. + git fetch origin +refs/heads/main:refs/remotes/origin/main HEAD_MSG=$(git log -1 --pretty=%s HEAD) if ! printf '%s' "$HEAD_MSG" | grep -q '^chore(release):'; then - echo "::error::Recovery requires HEAD to be a chore(release): commit. HEAD subject is: $HEAD_MSG" + # Escape `%`, CR, LF before embedding user-controlled commit + # subject in a workflow command — per GitHub's + # workflow-commands docs, those bytes can corrupt the + # command payload or inject additional commands. + HEAD_MSG_ESCAPED="${HEAD_MSG//%/%25}" + HEAD_MSG_ESCAPED="${HEAD_MSG_ESCAPED//$'\r'/%0D}" + HEAD_MSG_ESCAPED="${HEAD_MSG_ESCAPED//$'\n'/%0A}" + echo "::error::Recovery requires HEAD to be a chore(release): commit. HEAD subject is: $HEAD_MSG_ESCAPED" echo "::error::If the failed release commit is no longer at HEAD, resolve manually — do not use this recovery path." exit 1 fi From 1e0012ba19bc2831d5e59d5d1976d8b87d5dccea Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 20:51:01 -0600 Subject: [PATCH 05/16] =?UTF-8?q?fix(release):=20round-5=20=E2=80=94=20anc?= =?UTF-8?q?hor=20BREAKING=20CHANGE=20body=20regex=20to=20footer=20position?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 ensemble caught a real bump-classification bug, empirically verified against this branch's own commits: `isBreaking()` body-scan used `\bBREAKING[- ]CHANGE\b`, matching the phrase anywhere in the commit body. That includes narrative text, docstring examples, and any commit that documents what a breaking-change footer is. Concrete failure: commit `f2ec77a` on this branch has a docstring example in its body that reads `BREAKING CHANGE: consumers must update their extends path`. Under the loose body regex, that triggers `isBreaking() === true` and forces a minor bump for the whole release — even though every commit on the branch is `fix(release): ...` (patch only). Per Conventional Commits spec, `BREAKING CHANGE:` (or `BREAKING-CHANGE:`) is a footer line. Anchor the regex with `^BREAKING[- ]CHANGE: ` and the multiline flag so only line-anchored occurrences match. Narrative mentions and indented examples no longer false-positive. Verified on this branch's 4 commits: all now classify as patch (PATCH verdict), matching the intended `fix:` semantics. --- scripts/auto-changeset.ts | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/scripts/auto-changeset.ts b/scripts/auto-changeset.ts index b66075b..4b5e96d 100644 --- a/scripts/auto-changeset.ts +++ b/scripts/auto-changeset.ts @@ -139,13 +139,19 @@ function hasManualChangesets(): boolean { function isBreaking(commit: Commit): boolean { // `type!: ...` or `type(scope)!: ...` if (/^[a-z]+(\([^)]+\))?!:/.test(commit.subject)) return true; - // `BREAKING CHANGE:` or `BREAKING-CHANGE:` in the subject OR body. - // The Conventional Commits spec places these in a footer, so the - // body check is the canonical case; subject check catches the (less - // common) inline form. - const breakingPattern = /\bBREAKING[- ]CHANGE\b/; - if (breakingPattern.test(commit.subject)) return true; - if (breakingPattern.test(commit.body)) return true; + // Per Conventional Commits spec, `BREAKING CHANGE:` (or + // `BREAKING-CHANGE:`) is a footer line — at the start of its own + // line, followed by `: `. Anchoring with `^…: ` (multiline) is + // required to avoid false positives from narrative or docstring + // mentions of the literal phrase. Without this anchor, a body that + // SHOWS a `BREAKING CHANGE:` example (like this script's own + // top-of-file JSDoc, or any commit explaining what a breaking-change + // footer is) would silently force a minor bump. + const footerPattern = /^BREAKING[- ]CHANGE: /m; + if (footerPattern.test(commit.body)) return true; + // Catch the same footer accidentally written in the subject line. + // Rare but unambiguous intent — treat it as breaking. + if (/^BREAKING[- ]CHANGE: /.test(commit.subject)) return true; return false; } From f4608545d86363afb86674553b13cdfebb48825d Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 20:56:52 -0600 Subject: [PATCH 06/16] =?UTF-8?q?fix(release):=20round-6=20=E2=80=94=20res?= =?UTF-8?q?tore=20inline=20subject=20BREAKING=20CHANGE=20matching?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 tightened the subject regex to `^BREAKING[- ]CHANGE: ` (line start anchor), matching the body regex. That fixed the docstring-false- positive but accidentally narrowed too far: the previously documented inline subject form `feat: BREAKING CHANGE: remove legacy preset` no longer matches, so a breaking release could ship under a patch version. Failure path: - Subject `feat: BREAKING CHANGE: remove legacy preset` - `!` regex on line 141 — no match (`feat:` not `feat!:`) - body footer regex — no match (no body footer) - subject `^BREAKING[- ]CHANGE: ` — no match (starts with `feat:`) - `isBreaking()` → false → patch bump for a breaking change Fix: keep the body regex line-anchored (footer-correct) but loosen the subject regex back to `\bBREAKING[- ]CHANGE: ` — word boundary + colon-space. The colon requirement keeps narrative phrasing like "avoid BREAKING CHANGE false positives" out; the word-boundary lets the marker appear anywhere in the subject including after `feat:` / `fix:` prefixes. Verified 8 edge cases (5 subject + 3 body) including indented docstring examples, narrative prose, hyphenated variants, and the restored inline subject form. All pass. --- scripts/auto-changeset.ts | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/scripts/auto-changeset.ts b/scripts/auto-changeset.ts index 4b5e96d..cfea603 100644 --- a/scripts/auto-changeset.ts +++ b/scripts/auto-changeset.ts @@ -141,17 +141,20 @@ function isBreaking(commit: Commit): boolean { if (/^[a-z]+(\([^)]+\))?!:/.test(commit.subject)) return true; // Per Conventional Commits spec, `BREAKING CHANGE:` (or // `BREAKING-CHANGE:`) is a footer line — at the start of its own - // line, followed by `: `. Anchoring with `^…: ` (multiline) is - // required to avoid false positives from narrative or docstring - // mentions of the literal phrase. Without this anchor, a body that - // SHOWS a `BREAKING CHANGE:` example (like this script's own - // top-of-file JSDoc, or any commit explaining what a breaking-change - // footer is) would silently force a minor bump. - const footerPattern = /^BREAKING[- ]CHANGE: /m; - if (footerPattern.test(commit.body)) return true; - // Catch the same footer accidentally written in the subject line. - // Rare but unambiguous intent — treat it as breaking. - if (/^BREAKING[- ]CHANGE: /.test(commit.subject)) return true; + // line in the body, followed by `: `. Anchoring with `^…: ` + // (multiline) avoids false positives from narrative or docstring + // mentions. Without this anchor, a body that SHOWS a + // `BREAKING CHANGE:` example (like this script's own top-of-file + // JSDoc, or any commit explaining what a breaking-change footer is) + // would silently force a minor bump. + if (/^BREAKING[- ]CHANGE: /m.test(commit.body)) return true; + // Subject: looser anchor — require the `: ` separator (so narrative + // phrasing like "fix: avoid BREAKING CHANGE false positives" stays + // out) but allow the marker anywhere within the subject. Catches + // both the bare form (`BREAKING CHANGE: …`) and the inline form + // (`feat: BREAKING CHANGE: …`). Subjects are short and single-line + // so line-anchoring would be unnecessarily strict. + if (/\bBREAKING[- ]CHANGE: /.test(commit.subject)) return true; return false; } From 7e13016219c1f76e78ce13ac8a461b83d099bade Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 21:00:43 -0600 Subject: [PATCH 07/16] =?UTF-8?q?refactor(release):=20drop=20BREAKING=20CH?= =?UTF-8?q?ANGE=20footer=20scanning=20=E2=80=94=20rely=20on=20!=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rounds 5 and 6 of the review loop were both bug-fixing the same feature — and that feature does not match how this org actually writes commits. The org's convention is `feat!:` / `feat(scope)!:`, not `BREAKING CHANGE:` footers. Footer scanning is a regex-tuning trap: - Round 5 caught the body regex matching docstring narrative (`f2ec77a`'s own JSDoc example would have triggered a false minor bump for this very PR). - Round 6 caught the round-5 tightening dropping the legitimate inline subject form (`feat: BREAKING CHANGE: ...`). Every fix for one false positive introduces a window for a different false negative — narrative vs footer, indented vs unindented, inline subject vs bare subject, hyphenated vs spaced. None of those edge cases would arise if we simply recognized `!`. Drop footer scanning entirely. `isBreaking()` is now one line. As a follow-on simplification, the body fetch goes away too: the git pretty format drops to `%H %s`, the field/record separator machinery disappears, and parsing uses newlines (git subjects can't contain newlines, so they're unambiguous). If a footer-marked commit ever lands (e.g. from an external contributor unfamiliar with the org convention), the documented escape hatch is to drop a manual `.changeset/*.md` file — the deferral check at the top of main() yields to it. Net: -51 / +27 lines, removes the two highest-friction regex paths in the file. --- scripts/auto-changeset.ts | 78 ++++++++++++++------------------------- 1 file changed, 27 insertions(+), 51 deletions(-) diff --git a/scripts/auto-changeset.ts b/scripts/auto-changeset.ts index cfea603..0de3d20 100644 --- a/scripts/auto-changeset.ts +++ b/scripts/auto-changeset.ts @@ -12,19 +12,16 @@ * see in the changelog. * * Bump rules for 0.x.x releases (everything stays pre-1.0): - * - Any commit with `!` after type/scope, or `BREAKING CHANGE` / - * `BREAKING-CHANGE` in the subject OR body → minor bump + * - Any commit with `!` after type/scope (e.g. `feat!:` or + * `feat(scope)!:`) → minor bump * - All other commits (including `docs:`, `chore:`, `ci:`) → patch bump * - * Body inspection matters: the Conventional Commits spec puts breaking - * markers in a footer, e.g. - * - * feat: change config shape - * - * BREAKING CHANGE: consumers must update their extends path - * - * Subject-only inspection would classify this as a patch and ship a - * breaking change under a patch version. We scan the body too. + * We deliberately do NOT scan for `BREAKING CHANGE:` / `BREAKING-CHANGE:` + * footers. The org's convention is `!`, and footer scanning is a + * regex-tuning trap (narrative mentions, docstring examples, and inline + * subject forms each need bespoke handling). If a footer-marked commit + * ever lands here, the maintainer can drop a manual changeset to force + * the right bump — the deferral path is the documented escape hatch. * * `chore(release):` commits are filtered out — they're the workflow's * own version-bump commits and including them would re-bump on the @@ -45,20 +42,13 @@ const PACKAGES = [ interface Commit { hash: string; subject: string; - body: string; } -// Field/record separators for parsing `git log` output. We need bytes -// that won't appear in commit subjects or bodies and that don't break -// Node's child_process argv handling. Git's `%x` pretty-format -// escape emits the byte in the OUTPUT without requiring us to put it -// in the argv (which would fail — Node rejects NUL bytes in args, and -// even non-NUL control bytes in argv are awkward). We use ASCII 31 -// (Unit Separator) between fields and ASCII 30 (Record Separator) -// between commits — neither appears in real commit text. -const FIELD_SEP_OUT = '\x1f'; -const COMMIT_SEP_OUT = '\x1e'; -const GIT_PRETTY_FORMAT = `%H%x1f%s%x1f%b%x1e`; +// Subject-only output: hash + space + subject + newline per commit. +// We don't fetch the body (we don't scan footers — see top-of-file +// JSDoc), so newlines are safe as record separators: git commit +// subjects can't contain newlines. +const GIT_PRETTY_FORMAT = `%H %s`; /** * Run a git subcommand with args passed positionally. Uses execFileSync @@ -109,19 +99,21 @@ function getLastReleaseTag(): string | null { function getCommitsSinceLastRelease(): Commit[] { const lastTag = getLastReleaseTag(); const range = lastTag ? `${lastTag}..HEAD` : 'HEAD'; - // %x1f / %x1e are git pretty-format escapes that emit ASCII 31 / 30 - // in the OUTPUT. We don't put those bytes in argv (Node rejects NUL - // bytes there, and embedding raw control bytes is brittle); git - // expands them when generating the log. const log = git('log', range, `--pretty=format:${GIT_PRETTY_FORMAT}`, '--no-merges'); if (!log) return []; + // Split on newlines — git subjects can't contain newlines, so + // there's no ambiguity. First space splits hash from subject. return log - .split(COMMIT_SEP_OUT) - .map((entry) => entry.trim()) + .split('\n') + .map((line) => line.trim()) .filter(Boolean) - .map((entry) => { - const [hash = '', subject = '', body = ''] = entry.split(FIELD_SEP_OUT); - return { hash, subject, body }; + .map((line) => { + const spaceIdx = line.indexOf(' '); + if (spaceIdx === -1) return { hash: line, subject: '' }; + return { + hash: line.slice(0, spaceIdx), + subject: line.slice(spaceIdx + 1), + }; }); } @@ -137,25 +129,9 @@ function hasManualChangesets(): boolean { } function isBreaking(commit: Commit): boolean { - // `type!: ...` or `type(scope)!: ...` - if (/^[a-z]+(\([^)]+\))?!:/.test(commit.subject)) return true; - // Per Conventional Commits spec, `BREAKING CHANGE:` (or - // `BREAKING-CHANGE:`) is a footer line — at the start of its own - // line in the body, followed by `: `. Anchoring with `^…: ` - // (multiline) avoids false positives from narrative or docstring - // mentions. Without this anchor, a body that SHOWS a - // `BREAKING CHANGE:` example (like this script's own top-of-file - // JSDoc, or any commit explaining what a breaking-change footer is) - // would silently force a minor bump. - if (/^BREAKING[- ]CHANGE: /m.test(commit.body)) return true; - // Subject: looser anchor — require the `: ` separator (so narrative - // phrasing like "fix: avoid BREAKING CHANGE false positives" stays - // out) but allow the marker anywhere within the subject. Catches - // both the bare form (`BREAKING CHANGE: …`) and the inline form - // (`feat: BREAKING CHANGE: …`). Subjects are short and single-line - // so line-anchoring would be unnecessarily strict. - if (/\bBREAKING[- ]CHANGE: /.test(commit.subject)) return true; - return false; + // `type!: ...` or `type(scope)!: ...` — the only breaking marker + // we recognize. See top-of-file JSDoc for why we don't scan footers. + return /^[a-z]+(\([^)]+\))?!:/.test(commit.subject); } function main(): void { From 56fdf9f4c1bc6670ee0fe2355070b4a4dca3b335 Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 21:10:32 -0600 Subject: [PATCH 08/16] =?UTF-8?q?fix(release):=20round-7=20=E2=80=94=20emp?= =?UTF-8?q?ty-changeset=20gate=20+=20throw=20on=20Node-level=20git=20error?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-7 ensemble found two more correctness issues: 1. [medium] Empty-changeset trap `pnpm changeset:version` reads and DELETES changeset files even when their frontmatter is empty (no bumps). The old gate `git diff --quiet && git diff --cached --quiet` would see the deletion as a change, commit it, run publish as a no-op, advance no tags. The next auto run would then re-scan commits from the same (unmoved) tag boundary and re-include the commits the empty changeset was meant to skip. Fix: scope the gate to `packages/*/package.json` only. If no package version actually bumped, exit clean — leave the deleted changeset file uncommitted on the runner (next push re-fetches it from origin anyway). 2. [low] Node-level git errors went silent The round-2 `git()` helper logged Node-level failures (NUL in argv, missing binary, spawn errors) but still returned empty. Callers interpret empty as "no commits" / "no tags" — a real spawn-time bug would skip the release with a green workflow run. Fix: throw on Node-level errors (e.status undefined/null), keep the empty-return path only for git's own non-zero exits (which are expected for `describe --tags` with no tags). Verified: NUL in argv now throws ERR_INVALID_ARG_VALUE rather than silently returning. --- .github/workflows/release.yml | 25 +++++++++++++++---------- scripts/auto-changeset.ts | 29 +++++++++++++++++++---------- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d63e951..dc4381f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -98,16 +98,21 @@ jobs: # CHANGELOG.md entries + deletes consumed .changeset/*.md. pnpm changeset:version - # 3. Gate on whether anything actually changed. Using the - # working tree (not a single package's version) catches - # every case: auto-generated bumps to all packages, - # manual changesets that bump only one package, no-op - # runs where nothing was releasable. Avoids the - # single-package gate trap (manual changeset for only - # prettier-config would have skipped publish under the - # old eslint-only check). - if git diff --quiet && git diff --cached --quiet; then - echo "No releasable changes after changeset:version — nothing to publish." + # 3. Gate on whether any package.json version actually + # changed. Scoping the diff to `packages/*/package.json` + # (rather than the whole working tree) catches both: + # - subset bumps (manual changeset for only one package + # bumps that one's package.json; the old single-package + # gate for eslint-config would have skipped) + # - empty changesets (a manual `---\n---` changeset + # causes `changeset version` to delete the file but + # not bump any package.json; a working-tree-wide gate + # would have committed the deletion + run publish as + # a no-op + advanced no tag, leaving the next auto + # run to re-include the same commits) + if git diff --quiet -- 'packages/*/package.json' \ + && git diff --cached --quiet -- 'packages/*/package.json'; then + echo "No package.json bumps after changeset:version — nothing to publish." exit 0 fi diff --git a/scripts/auto-changeset.ts b/scripts/auto-changeset.ts index 0de3d20..6a2989d 100644 --- a/scripts/auto-changeset.ts +++ b/scripts/auto-changeset.ts @@ -52,13 +52,18 @@ const GIT_PRETTY_FORMAT = `%H %s`; /** * Run a git subcommand with args passed positionally. Uses execFileSync - * so no shell parsing occurs. Returns trimmed stdout, or empty string - * when git itself exits non-zero (e.g. `git describe` with no tags). + * so no shell parsing occurs. Returns trimmed stdout. * - * Surfaces non-git errors (invalid argv, missing binary, etc.) by - * logging to stderr and returning empty — we don't want a typo in our - * own format string to look like "no commits" and silently skip a - * release. Caller should treat empty as "no output" but log if needed. + * Two failure modes are distinguished: + * - Git exited non-zero (e.g. `describe --tags` with no tags exists). + * Returns empty string; callers treat empty as "no output". + * - Node-level failure (invalid argv, binary missing, spawn error). + * This indicates a bug in OUR code, not git's expected behaviour, + * so we throw to fail the workflow loudly. The previous "log and + * return empty" path made a typo in our own format string look + * identical to "no commits since last release" — auto-changeset + * would silently skip the run and the workflow would go green + * without producing a release. */ function git(...args: string[]): string { try { @@ -67,13 +72,17 @@ function git(...args: string[]): string { stdio: ['pipe', 'pipe', 'pipe'], }).trim(); } catch (err) { - // Git returning non-zero is expected for some calls (describe with - // no tags). But Node-level errors (NUL in args, binary not found, - // etc.) indicate our bug, not git's — surface them. const e = err as { status?: number | null; code?: string; message?: string }; if (e.status === undefined || e.status === null) { - console.error(`auto-changeset: git ${args.join(' ')} failed: ${e.code ?? e.message ?? err}`); + // Node-level error — re-throw to fail loudly. We never want + // this swallowed; an empty return here would let the caller + // proceed as if git succeeded with no output. + throw new Error( + `auto-changeset: git ${args.join(' ')} failed at Node level: ${e.code ?? e.message ?? err}`, + ); } + // Git's own non-zero exit — expected for `describe --tags` + // when no tags exist, etc. Empty output signals that. return ''; } } From 924621c0229f611552f4609d7d6ac25fbeddbe04 Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 21:18:07 -0600 Subject: [PATCH 09/16] =?UTF-8?q?fix(release):=20round-8=20=E2=80=94=20par?= =?UTF-8?q?tial-release=20detection=20+=20cleanup=20push=20+=20accurate=20?= =?UTF-8?q?commit=20title?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-8 ensemble found three issues, including a regression from round-7's empty-changeset fix: 1. [medium] Tag-push failures lost the release boundary If `pnpm changeset:publish` succeeds but `git push origin --tags` fails, packages are on the registry, the commit is on main, but the per-package tags are not on origin. The recovery path requires HEAD to still be the `chore(release):` commit — once any new commit lands, recovery is blocked, and the next auto run scans `LAST_TAG..HEAD`, filters out the chore commit, and re-includes the commits that were already published. Duplicate release with duplicate changelog entries. Fix: add a step-0 guard that scans for an untagged `chore(release):` commit between the last `@happyvertical/*@*` tag and HEAD. If found, abort with an operator-action message pointing at the resolution (hand-push the missing tags, then re-run). 2. [medium] No-op changesets were stuck on main (regression from round-7) Round-7's package-json gate exits early without committing the deletion of an empty manual changeset. The cleanup never reaches origin, so the no-op changeset stays on main — every future push repeats the same no-op cycle, silently blocking real auto-changeset runs. Fix: when the gate finds no package bumps but the tree changed, commit + push the cleanup with a `chore(release):` title (so the next push doesn't re-trigger us via the job-level guard). Includes the same origin-main-advanced check used in the main path to avoid drift. 3. [low] Subset releases got a misleading commit title The old `chore(release): v$AFTER` used eslint-config's version unconditionally. A manual changeset that bumps only prettier-config would commit `v` while actually publishing a different package. Fix: build the commit title from the package names that actually bumped this run (`git diff --name-only -- 'packages/*/package.json'` → comma-separated list). Per-package versions live in the tags created by `changeset publish`; the commit title carries which packages changed. --- .github/workflows/release.yml | 55 +++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index dc4381f..67deb08 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -90,6 +90,26 @@ jobs: run: | set -euo pipefail + # 0. Refuse to start if a previous release didn't complete + # its tag push. Symptom: an unreleased `chore(release):` + # commit sits between the last `@happyvertical/*@*` tag + # and HEAD. If we proceeded, `auto-changeset` would scan + # `LAST_TAG..HEAD`, filter out the chore commit, and + # re-include the commits that were already published — + # causing a duplicate release with duplicated changelog + # entries. Operator resolution: hand-push the missing + # per-package tags for the already-published versions, + # then re-run this workflow. + LAST_PKG_TAG=$(git tag --list '@happyvertical/*@*' --sort=-creatordate | head -1 || true) + if [ -n "$LAST_PKG_TAG" ]; then + UNTAGGED_RELEASE=$(git log "$LAST_PKG_TAG..HEAD" --pretty=%H --grep='^chore(release):' | head -1) + if [ -n "$UNTAGGED_RELEASE" ]; then + echo "::error::Found untagged chore(release): commit $UNTAGGED_RELEASE since last per-package tag $LAST_PKG_TAG." + echo "::error::This indicates the previous release published packages but failed to push tags. Resolve manually: push the missing per-package tags for the published versions, then re-run." + exit 1 + fi + fi + # 1. Generate a changeset from conventional commits since the # last release (or defer if a manual changeset is present). pnpm changeset:auto @@ -112,20 +132,43 @@ jobs: # run to re-include the same commits) if git diff --quiet -- 'packages/*/package.json' \ && git diff --cached --quiet -- 'packages/*/package.json'; then + # No package bumped — but `changeset:version` may have + # consumed (deleted) a manual no-op changeset. If we exit + # here without committing that deletion, the no-op + # changeset stays on origin/main, and EVERY future push + # repeats the same no-op cycle, silently blocking real + # releases until someone notices and deletes it by hand. + # Push the cleanup commit so the blocker is gone. + if ! git diff --quiet || ! git diff --cached --quiet; then + echo "No package bumps but tree changed — committing cleanup of consumed no-op changesets." + git add -A + git commit -m "chore(release): clean up consumed no-op changesets" + git fetch origin +refs/heads/main:refs/remotes/origin/main + if [ "$(git rev-parse origin/main)" != "$(git rev-parse HEAD~1)" ]; then + echo "::error::origin/main advanced during this run — aborting cleanup push." + exit 1 + fi + git push origin main + fi echo "No package.json bumps after changeset:version — nothing to publish." exit 0 fi - # Use eslint-config as a representative version for logging. - # All publishable packages bump together under auto-changeset - # (manual changesets may bump a subset). - AFTER=$(node -p "require('./packages/eslint-config/package.json').version") - echo "Releasing version $AFTER" + # Identify which packages actually bumped this run. Used + # both for logging and for the commit message — naming a + # specific version (like `v$ESLINT_VERSION`) would be wrong + # for subset releases via manual changesets that bump only + # one or two packages. Per-package versions live in the + # tags created by `changeset publish`. + BUMPED=$(git diff --name-only -- 'packages/*/package.json' \ + | sed 's|packages/\(.*\)/package.json|\1|' \ + | paste -sd ',' -) + echo "Bumped packages: $BUMPED" # 4. Commit the version bumps + changelog entries. The chore # prefix matches what the job-level `if:` guard filters. git add -A - git commit -m "chore(release): v$AFTER" + git commit -m "chore(release): bump $BUMPED" # 5. Verify main hasn't moved out from under us, then push # the commit BEFORE publishing. Order matters: if publish From e4d2b5362fe1d2ef2360887a7ad161ba93a0a5bf Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 21:23:24 -0600 Subject: [PATCH 10/16] =?UTF-8?q?fix(release):=20round-9=20=E2=80=94=20nar?= =?UTF-8?q?row=20partial-release=20detector=20to=20`bump`=20commits=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-9 caught a regression where round-8's two fixes interacted badly: - Fix 8.1 added a preflight detecting any `chore(release):` commit between the last per-package tag and HEAD (signal of failed tag-push from a previous release). - Fix 8.2 added a cleanup path that creates `chore(release): clean up consumed no-op changesets` commits when a manual no-op changeset needs to be removed from main. After a cleanup commit lands, the preflight grep matches it (same `chore(release):` prefix), and EVERY future release self-blocks forever — the operator sees "previous release didn't complete tag push" errors on commits that were never a release. Fix: narrow the preflight grep to `^chore(release): bump ` — the exact format the version-bump commit uses (set by fix 8.3). The cleanup commit's `clean up consumed no-op changesets` suffix doesn't match, so it's correctly ignored by the partial-release detector. Verified the grep behavior in a synthetic git repo: pattern matches `chore(release): bump eslint-config,prettier-config`, rejects `chore(release): clean up consumed no-op changesets` and `fix(release): not a release commit`. --- .github/workflows/release.yml | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 67deb08..86a5d0a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -91,7 +91,7 @@ jobs: set -euo pipefail # 0. Refuse to start if a previous release didn't complete - # its tag push. Symptom: an unreleased `chore(release):` + # its tag push. Symptom: an untagged `chore(release): bump` # commit sits between the last `@happyvertical/*@*` tag # and HEAD. If we proceeded, `auto-changeset` would scan # `LAST_TAG..HEAD`, filter out the chore commit, and @@ -100,11 +100,18 @@ jobs: # entries. Operator resolution: hand-push the missing # per-package tags for the already-published versions, # then re-run this workflow. + # + # Detector intentionally narrowed to `bump ` (the + # version-bump message format). The cleanup path below + # creates `chore(release): clean up consumed no-op + # changesets` commits which are NOT failed releases — + # matching them here would self-block every future run + # after any no-op cleanup ever lands on main. LAST_PKG_TAG=$(git tag --list '@happyvertical/*@*' --sort=-creatordate | head -1 || true) if [ -n "$LAST_PKG_TAG" ]; then - UNTAGGED_RELEASE=$(git log "$LAST_PKG_TAG..HEAD" --pretty=%H --grep='^chore(release):' | head -1) + UNTAGGED_RELEASE=$(git log "$LAST_PKG_TAG..HEAD" --pretty=%H --grep='^chore(release): bump ' | head -1) if [ -n "$UNTAGGED_RELEASE" ]; then - echo "::error::Found untagged chore(release): commit $UNTAGGED_RELEASE since last per-package tag $LAST_PKG_TAG." + echo "::error::Found untagged chore(release): bump commit $UNTAGGED_RELEASE since last per-package tag $LAST_PKG_TAG." echo "::error::This indicates the previous release published packages but failed to push tags. Resolve manually: push the missing per-package tags for the published versions, then re-run." exit 1 fi From fa28980f0d89cf647de1d6aa10dd556d5a20eb09 Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 21:30:06 -0600 Subject: [PATCH 11/16] =?UTF-8?q?fix(release):=20round-10=20=E2=80=94=20fi?= =?UTF-8?q?rst-release=20guard=20coverage=20+=20accurate=20recovery=20guid?= =?UTF-8?q?ance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 ensemble found two issues with the round-8 partial-release detector: 1. [HIGH] First-release partial publish bypassed the guard The detector only ran when `LAST_PKG_TAG` was non-empty. That excluded the exact case the guard is meant to catch in a fresh repo: a first release that publishes packages but fails to push tags. With no prior tags, `LAST_PKG_TAG` is empty, the guard is skipped, and `auto-changeset` scans all history, filters out the chore commit, and re-includes the commits that were already published — duplicate release. Fix: when no package tag exists, scan all history (`UNTAGGED_RANGE=HEAD`) instead of skipping the guard. Same detection, same abort path. 2. [medium] Error message could lead operators to publish-skip The old message said "previous release published packages but failed to push tags" and told operators to push the missing tags. But the same untagged-bump-commit state can arise from a partial publish failure (some packages published, some didn't) before tag push was even attempted. If an operator follows the old guidance and pushes tags for versions that don't exist on the registry, future auto runs treat those tags as the release boundary and skip publishing the missing versions — silent data loss. Fix: distinguish the two failure modes explicitly. Recovery guidance now: (a) Recommends workflow_dispatch publish-only when HEAD is still the bump commit. That path is idempotent on both axes: changeset:publish skips already-published versions, and changeset tag creates tags only for versions in package.json. Single action covers both partial-publish and tag-push-failure. (b) When HEAD has advanced past the bump commit (the case where this guard fires), tells operators to manually verify registry state before pushing any tags, with a concrete example of how to tag-only what's actually published. --- .github/workflows/release.yml | 46 ++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 86a5d0a..3cb0f87 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -92,29 +92,37 @@ jobs: # 0. Refuse to start if a previous release didn't complete # its tag push. Symptom: an untagged `chore(release): bump` - # commit sits between the last `@happyvertical/*@*` tag - # and HEAD. If we proceeded, `auto-changeset` would scan - # `LAST_TAG..HEAD`, filter out the chore commit, and - # re-include the commits that were already published — - # causing a duplicate release with duplicated changelog - # entries. Operator resolution: hand-push the missing - # per-package tags for the already-published versions, - # then re-run this workflow. + # commit exists since the last `@happyvertical/*@*` tag + # (or anywhere in history if no package tags exist yet — + # that's the first-release-partial-failure case). If we + # proceeded, `auto-changeset` would scan that range, + # filter out the chore commit, and re-include the + # commits that were already published — causing a + # duplicate release with duplicated changelog entries. # # Detector intentionally narrowed to `bump ` (the - # version-bump message format). The cleanup path below - # creates `chore(release): clean up consumed no-op - # changesets` commits which are NOT failed releases — - # matching them here would self-block every future run - # after any no-op cleanup ever lands on main. + # version-bump message format set by step 4 below). The + # cleanup path further down creates `chore(release): + # clean up consumed no-op changesets` commits which are + # NOT failed releases — matching them here would + # self-block every future run after any cleanup landed. LAST_PKG_TAG=$(git tag --list '@happyvertical/*@*' --sort=-creatordate | head -1 || true) if [ -n "$LAST_PKG_TAG" ]; then - UNTAGGED_RELEASE=$(git log "$LAST_PKG_TAG..HEAD" --pretty=%H --grep='^chore(release): bump ' | head -1) - if [ -n "$UNTAGGED_RELEASE" ]; then - echo "::error::Found untagged chore(release): bump commit $UNTAGGED_RELEASE since last per-package tag $LAST_PKG_TAG." - echo "::error::This indicates the previous release published packages but failed to push tags. Resolve manually: push the missing per-package tags for the published versions, then re-run." - exit 1 - fi + UNTAGGED_RANGE="$LAST_PKG_TAG..HEAD" + else + # No package tags exist yet. Scan all history — covers the + # first-release case where the initial publish/tag-push + # partially failed and left an untagged bump commit. + UNTAGGED_RANGE="HEAD" + fi + UNTAGGED_RELEASE=$(git log "$UNTAGGED_RANGE" --pretty=%H --grep='^chore(release): bump ' | head -1) + if [ -n "$UNTAGGED_RELEASE" ]; then + echo "::error::Found untagged chore(release): bump commit $UNTAGGED_RELEASE in range $UNTAGGED_RANGE." + echo "::error::The previous release left an unfinished state — either changeset:publish or the tag push failed before completing." + echo "::error::Recovery:" + echo "::error:: 1. If HEAD is still that bump commit: trigger workflow_dispatch with mode=publish-only. This re-runs changeset:publish (idempotent — skips already-published versions) AND changeset tag (creates missing per-package tags) AND pushes tags. Single action handles both failure modes." + echo "::error:: 2. If HEAD has advanced past it (likely, since this guard is firing): you cannot use publish-only recovery. Manually verify on npm.pkg.github.com which package versions actually published, hand-create + push per-package tags ONLY for those (e.g. \`git tag '@happyvertical/eslint-config@0.2.0' $UNTAGGED_RELEASE && git push origin --tags\`). For packages that weren't published, run \`pnpm changeset publish\` against the bump commit from a clean checkout. Do NOT push tags for versions that don't exist on the registry — future runs would treat them as the release boundary and skip publishing those versions." + exit 1 fi # 1. Generate a changeset from conventional commits since the From 46fe1b687596efaa27874abff213250f8f66408b Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 21:35:25 -0600 Subject: [PATCH 12/16] =?UTF-8?q?fix(release):=20round-11=20=E2=80=94=20su?= =?UTF-8?q?bject-only=20matching=20for=20both=20release-commit=20guards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-11 ensemble caught two findings, both rooted in the same bug class: matching commit message contents (body) instead of just the subject. 1. [medium] Untagged-release detector matched commit bodies `git log --grep` searches the FULL commit message — subject and body. Round-8 added `--grep='^chore(release): bump '` thinking it would only match release commit subjects, but a regular commit whose body documents the literal string `chore(release): bump …` (e.g. as a code example, like several of my own commit messages on this PR) sets `UNTAGGED_RELEASE` and aborts every future run. Fix: pipe `git log --pretty='%H %s'` through awk to filter on the subject line only. Verified in a synthetic repo: false-positive commit (body mentions `chore(release): bump …`) was matched by `--grep` but correctly skipped by the awk pipeline; the real bump commit still matched. 2. [medium] Publish-only recovery accepted cleanup commits Round-3's recovery guard checked `^chore(release):` — too loose. The cleanup commit from round-8 (`chore(release): clean up consumed no-op changesets`) matches that prefix. If an operator runs publish-only while that cleanup is at HEAD, `changeset tag` creates per-package tags pointing at the cleanup SHA instead of the actual version-bump commit. Future release scans then trust the wrong commit as the release boundary. Fix: narrow to `^chore(release): bump ` — the exact format of the real version-bump commits, matching the detector in step 0. --- .github/workflows/release.yml | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3cb0f87..de91011 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -115,7 +115,13 @@ jobs: # partially failed and left an untagged bump commit. UNTAGGED_RANGE="HEAD" fi - UNTAGGED_RELEASE=$(git log "$UNTAGGED_RANGE" --pretty=%H --grep='^chore(release): bump ' | head -1) + # `git log --grep` matches the entire commit message including + # body, so a regular commit whose body documents the literal + # string `chore(release): bump ...` (e.g. as a code example + # in PR description) would false-positive. Pipe subjects + # through awk to filter against the SUBJECT line only. + UNTAGGED_RELEASE=$(git log "$UNTAGGED_RANGE" --pretty='%H %s' \ + | awk '/^[a-f0-9]+ chore\(release\): bump / {print $1; exit}') if [ -n "$UNTAGGED_RELEASE" ]; then echo "::error::Found untagged chore(release): bump commit $UNTAGGED_RELEASE in range $UNTAGGED_RANGE." echo "::error::The previous release left an unfinished state — either changeset:publish or the tag push failed before completing." @@ -246,19 +252,30 @@ jobs: # versions. # Guard: refuse to publish unless HEAD is the failed release's - # `chore(release):` commit AND nothing new has landed on - # origin/main. Without this, recovery could publish stale + # `chore(release): bump …` commit AND nothing new has landed + # on origin/main. Without this, recovery could publish stale # version numbers from `R` (the failed release commit) with # the source tree of any later commit `N` that landed in the # meantime — the registry artifact would silently contain # post-version-commit code. Recovery is only safe immediately # after a failed publish, before any new commits land. + # + # Pattern is narrowed to `bump ` specifically. The auto path + # creates two flavours of chore(release): commit — the actual + # version bump (`chore(release): bump `) and the no-op + # cleanup (`chore(release): clean up consumed no-op + # changesets`). Only the bump variant is a valid recovery + # target; running publish-only with the cleanup commit at + # HEAD would call `changeset tag` against a commit that + # doesn't actually contain version bumps, pinning tags to + # the wrong SHA. + # # Explicit destination refspec — see auto path above for why # `git fetch origin main` alone is not reliable for updating # `refs/remotes/origin/main` under all configurations. git fetch origin +refs/heads/main:refs/remotes/origin/main HEAD_MSG=$(git log -1 --pretty=%s HEAD) - if ! printf '%s' "$HEAD_MSG" | grep -q '^chore(release):'; then + if ! printf '%s' "$HEAD_MSG" | grep -q '^chore(release): bump '; then # Escape `%`, CR, LF before embedding user-controlled commit # subject in a workflow command — per GitHub's # workflow-commands docs, those bytes can corrupt the @@ -266,7 +283,7 @@ jobs: HEAD_MSG_ESCAPED="${HEAD_MSG//%/%25}" HEAD_MSG_ESCAPED="${HEAD_MSG_ESCAPED//$'\r'/%0D}" HEAD_MSG_ESCAPED="${HEAD_MSG_ESCAPED//$'\n'/%0A}" - echo "::error::Recovery requires HEAD to be a chore(release): commit. HEAD subject is: $HEAD_MSG_ESCAPED" + echo "::error::Recovery requires HEAD to be a 'chore(release): bump …' commit. HEAD subject is: $HEAD_MSG_ESCAPED" echo "::error::If the failed release commit is no longer at HEAD, resolve manually — do not use this recovery path." exit 1 fi From f57176dcb8db75dff86edab2d96769943fb02395 Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 21:43:26 -0600 Subject: [PATCH 13/16] =?UTF-8?q?fix(release):=20round-12=20=E2=80=94=20fa?= =?UTF-8?q?il=20loudly=20on=20empty=20changesets=20+=20atomic=20tag=20push?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-12 ensemble found two findings: 1. [medium] No-op cleanup path didn't preserve "skipped" boundary Round-7/8 added a path that commits the cleanup of an empty manual changeset (`chore(release): clean up consumed no-op changesets`) so the empty file wouldn't keep blocking future runs. But: - The cleanup commits no tag - auto-changeset.ts's `LAST_PKG_TAG` still points at the previous real release - The commits the operator intended to "skip release" via the empty changeset stay in the next auto run's scan range - Next push releases them anyway The cleanup path was treating empty changesets as a valid "skip release" signal that this workflow can't actually honor (no commit-range bookkeeping persists across runs). Fix: remove the cleanup machinery. Fail loudly when an empty changeset is detected. Operator's only valid actions are: (a) fill in package bumps in the changeset frontmatter, or (b) delete the empty changeset file. This also kills the round-9 fix's reason-to-exist (narrowing the detector to exclude `chore(release): clean up` commits) — those commits no longer get created. The narrowing stays as defense in depth. 2. [medium] Non-atomic tag push left partial state undetectable `git push origin --tags` pushes all local tags but is NOT atomic: a network failure mid-push can land some tags and leave others local-only. The step-0 untagged-bump detector relies on `LAST_PKG_TAG` being absent when the previous release's tags never reached origin — a partial push sets `LAST_PKG_TAG` to one of the freshly-pushed sibling tags, the detector treats the bump commit as already-released, and the missing tags persist silently. Fix: `git push --atomic origin --tags` in both auto and recovery paths. With atomic, either all tags reach origin or none do, so the detector's "no tag pushed" branch is the only possible partial-tag state. --- .github/workflows/release.yml | 74 +++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index de91011..2b2388a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -153,25 +153,36 @@ jobs: # run to re-include the same commits) if git diff --quiet -- 'packages/*/package.json' \ && git diff --cached --quiet -- 'packages/*/package.json'; then - # No package bumped — but `changeset:version` may have - # consumed (deleted) a manual no-op changeset. If we exit - # here without committing that deletion, the no-op - # changeset stays on origin/main, and EVERY future push - # repeats the same no-op cycle, silently blocking real - # releases until someone notices and deletes it by hand. - # Push the cleanup commit so the blocker is gone. + # No package versions bumped after changeset:version. + # + # Two sub-cases: + # (a) Nothing changed at all — auto-changeset deferred + # and had no commits to act on, OR no manual + # changeset existed. Legitimate no-op; exit clean. + # (b) Tree changed but no package.json did — almost + # certainly an empty manual changeset (frontmatter + # with no package bumps) was consumed by + # changeset:version. The deletion is in the tree but + # no release happened. + # + # Empty changesets are NOT supported as a "skip release" + # signal in this repo. auto-changeset.ts uses the most + # recent per-package tag as its lower bound; nothing + # advances that boundary on a no-op release. The next push + # would re-include the commits the empty changeset was + # meant to skip and release them anyway, defeating the + # operator's intent. + # + # Fail loudly on case (b) so the operator either fills in + # the bumps or deletes the empty changeset. if ! git diff --quiet || ! git diff --cached --quiet; then - echo "No package bumps but tree changed — committing cleanup of consumed no-op changesets." - git add -A - git commit -m "chore(release): clean up consumed no-op changesets" - git fetch origin +refs/heads/main:refs/remotes/origin/main - if [ "$(git rev-parse origin/main)" != "$(git rev-parse HEAD~1)" ]; then - echo "::error::origin/main advanced during this run — aborting cleanup push." - exit 1 - fi - git push origin main + echo "::error::Tree changed but no package.json bumped — likely an empty manual changeset was consumed." + echo "::error::Empty/no-release changesets are not supported in this repo. Either add package bump lines to the changeset frontmatter (e.g. \"'@happyvertical/eslint-config': patch\") or delete the empty changeset file from .changeset/." + echo "::error::Files changed by changeset:version:" + git diff --name-only HEAD || true + exit 1 fi - echo "No package.json bumps after changeset:version — nothing to publish." + echo "No releasable changes after changeset:version — nothing to publish." exit 0 fi @@ -230,7 +241,18 @@ jobs: # the interval, AFTER packages have already been published # to the registry. That'd leave registry/git drift the # publish-only recovery path is meant to avoid. - git push origin --tags + # + # `--atomic` makes the push all-or-nothing for the set of + # refs included. Without it, a partial network/auth + # failure mid-push could land some per-package tags on + # origin and leave others local-only. The step-0 + # untagged-bump detector relies on LAST_PKG_TAG being + # NULL when no tags from a failed release reached origin; + # a partial tag push would set LAST_PKG_TAG to a freshly- + # pushed sibling tag, the detector would treat the range + # as already-released, and the missing tags would silently + # persist. + git push --atomic origin --tags - name: Publish only (recovery from partial publish failure) if: ${{ github.event_name == 'workflow_dispatch' && inputs.mode == 'publish-only' }} @@ -309,10 +331,12 @@ jobs: # would scan too far back and double-include old commits. pnpm changeset tag - # Push tags only — recovery never touches the main branch - # ref. The release commit was already pushed by the failed - # auto run; pushing main again here would only succeed if - # main hadn't advanced (which our guard above already - # ensures), so it'd be redundant. Explicit tag-only push - # keeps the intent clear. - git push origin --tags + # Push tags only, atomically — recovery never touches the + # main branch ref. The release commit was already pushed by + # the failed auto run; pushing main again here would only + # succeed if main hadn't advanced (which our guard above + # already ensures), so it'd be redundant. `--atomic` makes + # the tag push all-or-nothing so a partial push can't leave + # the repo in the same hidden-untagged-bump state we're + # trying to recover from. See auto path for full rationale. + git push --atomic origin --tags From 5169ee96a39fa76176875c15915457fd68665532 Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 22:05:57 -0600 Subject: [PATCH 14/16] =?UTF-8?q?fix(release):=20round-13=20=E2=80=94=20ga?= =?UTF-8?q?te=20detectors=20on=20bot-committer=20identity,=20not=20subject?= =?UTF-8?q?=20alone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-13 ensemble added copilot-cli (codex-cli had solo'd rounds 2-12). Copilot caught a real release-deadlock that codex missed across 12 rounds: The step-0 partial-release detector and the publish-only recovery guard both relied on subject pattern `^chore(release): bump ` to identify workflow-created release commits. But that subject is also a valid human Conventional Commit — e.g. `chore(release): bump pnpm/action-setup` for a dependency update. Deadlock path: - Human pushes `chore(release): bump pnpm/action-setup` - Job-level `if:` skips that push (filter on chore(release): prefix) - Any future push runs the workflow - Step-0 detector finds the human commit, treats it as a failed release commit, aborts with operator-action error - No release ever happens until someone manually deletes the human commit (impossible — it's already on main) or rewrites history Fix: combine subject pattern with committer-identity check. The workflow sets `have-release-bot` as the git committer for its own commits (line 81), so committer identity is a reliable machine-only marker that humans can't accidentally produce. - Step-0 detector: `git log --committer='have-release-bot' ...` combined with the existing awk subject filter - Recovery guard: also check `git log -1 --pretty=%cn HEAD` matches `have-release-bot`, in addition to the subject grep Verified in a synthetic repo: human commit with matching subject is correctly excluded; bot commit with same subject still matches. Process note: this finding shipped after rounds 2-12 of codex-cli solo because the loop wasn't running the ensemble it documents. From now: copilot-cli runs each round alongside codex-cli. Also rejecting copilot's second finding (loss of `BREAKING CHANGE` recognition) — that was the deliberate round-6.5 simplification (see commit 7e13016), not a regression. Copilot didn't see that design conversation. --- .github/workflows/release.yml | 43 ++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2b2388a..e1ecf74 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -115,12 +115,28 @@ jobs: # partially failed and left an untagged bump commit. UNTAGGED_RANGE="HEAD" fi - # `git log --grep` matches the entire commit message including - # body, so a regular commit whose body documents the literal - # string `chore(release): bump ...` (e.g. as a code example - # in PR description) would false-positive. Pipe subjects - # through awk to filter against the SUBJECT line only. - UNTAGGED_RELEASE=$(git log "$UNTAGGED_RANGE" --pretty='%H %s' \ + # Subject + committer identity together — subject alone is + # ambiguous because a human commit like `chore(release): + # bump pnpm/action-setup` is a valid Conventional Commit + # that matches the bump pattern but is NOT a failed release + # (it's a dependency-bump chore). On its own push it's + # skipped by the job-level chore(release) filter; the NEXT + # push then hits this detector, exits 1, no boundary + # advances, and every future release deadlocks until an + # operator manually fixes it. + # + # The bot identity (`have-release-bot`, set above) is set + # only by THIS workflow. Filtering by both subject AND + # committer means only workflow-created commits can trip + # the detector — human authors can't accidentally produce + # the marker even with a matching subject. + # + # `git log --grep` would match commit bodies (subject + + # body), not just subjects, so we use `--committer` for + # identity but pipe `%s` through awk for subject matching. + UNTAGGED_RELEASE=$(git log "$UNTAGGED_RANGE" \ + --committer='have-release-bot' \ + --pretty='%H %s' \ | awk '/^[a-f0-9]+ chore\(release\): bump / {print $1; exit}') if [ -n "$UNTAGGED_RELEASE" ]; then echo "::error::Found untagged chore(release): bump commit $UNTAGGED_RELEASE in range $UNTAGGED_RANGE." @@ -297,7 +313,16 @@ jobs: # `refs/remotes/origin/main` under all configurations. git fetch origin +refs/heads/main:refs/remotes/origin/main HEAD_MSG=$(git log -1 --pretty=%s HEAD) - if ! printf '%s' "$HEAD_MSG" | grep -q '^chore(release): bump '; then + HEAD_COMMITTER=$(git log -1 --pretty=%cn HEAD) + # Check subject AND committer identity. Subject alone is + # ambiguous: `chore(release): bump pnpm/action-setup` is a + # legit human conventional commit that would otherwise pass + # this guard and reach `pnpm changeset:publish` against the + # wrong source tree. The bot identity is set only by this + # workflow's auto step, so it's a reliable machine-only + # marker for "commit was created by the auto release path". + if ! printf '%s' "$HEAD_MSG" | grep -q '^chore(release): bump ' \ + || [ "$HEAD_COMMITTER" != "have-release-bot" ]; then # Escape `%`, CR, LF before embedding user-controlled commit # subject in a workflow command — per GitHub's # workflow-commands docs, those bytes can corrupt the @@ -305,7 +330,9 @@ jobs: HEAD_MSG_ESCAPED="${HEAD_MSG//%/%25}" HEAD_MSG_ESCAPED="${HEAD_MSG_ESCAPED//$'\r'/%0D}" HEAD_MSG_ESCAPED="${HEAD_MSG_ESCAPED//$'\n'/%0A}" - echo "::error::Recovery requires HEAD to be a 'chore(release): bump …' commit. HEAD subject is: $HEAD_MSG_ESCAPED" + echo "::error::Recovery requires HEAD to be a 'chore(release): bump …' commit by have-release-bot." + echo "::error::HEAD subject: $HEAD_MSG_ESCAPED" + echo "::error::HEAD committer: $HEAD_COMMITTER" echo "::error::If the failed release commit is no longer at HEAD, resolve manually — do not use this recovery path." exit 1 fi From c863216112e168f46b021b6503749ec59c1f5de4 Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 22:14:47 -0600 Subject: [PATCH 15/16] =?UTF-8?q?fix(release):=20round-14=20=E2=80=94=20bo?= =?UTF-8?q?t-committer=20filter=20in=20job=20gate=20+=20auto-changeset=20+?= =?UTF-8?q?=20escape=20HEAD=5FCOMMITTER?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-14 ensemble (codex-cli + copilot-cli both running this round — correcting rounds 2-13 which were codex-cli solo). Copilot caught two real findings codex missed; codex caught one of them at lower severity. Deduplicated: 1. [medium] Subject-only chore(release) match in TWO more code paths (extension of round-13 finding into job gate + auto-changeset) Round-13 fixed the step-0 detector and the publish-only recovery guard to require BOTH subject pattern AND `have-release-bot` committer identity. But two other places still used subject-only matching: - Job-level `if:` filter (line 42): skips ANY commit starting with `chore(release):` regardless of committer. A human commit like `chore(release): bump pnpm/action-setup` (valid Conventional Commit for a dependency update) would never trigger the release workflow for its own changes. - auto-changeset.ts filter (line 161): excludes ANY commit with that subject prefix from the generated changelog summary. The same human commit would be silently absent from release notes even when a subsequent commit triggers a release. Net effect: human-authored `chore(release):` commits become invisible in release pipeline AND release notes. Fix: - Job-level if: combine `startsWith` with `github.event.head_commit.committer.name == 'have-release-bot'`. GitHub's push event payload includes committer.name so we can gate at the workflow level without running any steps. - auto-changeset.ts: re-add committer field to git log output (using `%cn` with `\x1f` field separator — newline still separates records since subjects/committers can't contain newlines). Update the filter to require BOTH conditions. Verified in synthetic git repos that human commits with matching subject pass through the filter while bot commits with matching subject are excluded. 2. [medium] HEAD_COMMITTER printed in ::error:: without escaping (caught by both reviewers, copilot rated medium / codex rated low) Round-13 added HEAD_COMMITTER to the recovery guard's failure output but only escaped HEAD_MSG. Copilot empirically verified that git accepts committer names containing `%0A::warning::pwn` — direct workflow-command injection risk. Fix: extract escape logic into a local `escape_wc` function and apply to BOTH user-controlled git metadata fields before embedding in workflow commands. Process note: the round-1-13 codex-cli-solo run produced 22 findings across 12 rounds. Copilot's first-round catch on a fundamental deadlock scenario (covered by round 13) and this round's two findings (both real) show why the ensemble is non-negotiable. Going forward: both run every round; ensemble dedup happens at fix time. --- .github/workflows/release.yml | 41 +++++++++++++++++++++-------- scripts/auto-changeset.ts | 49 +++++++++++++++++++++++++---------- 2 files changed, 65 insertions(+), 25 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e1ecf74..8c1562c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -35,11 +35,22 @@ jobs: name: Direct-publish runs-on: ubuntu-latest timeout-minutes: 15 - # Skip the workflow's own chore(release) push commits during the - # `push` event. Without this guard, every release commit would - # trigger another (no-op) run. workflow_dispatch invocations are + # Skip the workflow's own release commits during the `push` event. + # Without this guard, every workflow-created release commit would + # trigger another (no-op) run. `workflow_dispatch` invocations are # never skipped — that's the recovery path. - if: ${{ github.event_name == 'workflow_dispatch' || !startsWith(github.event.head_commit.message, 'chore(release):') }} + # + # Match on subject prefix AND committer identity together. Subject + # alone (`chore(release):`) is also a valid human Conventional + # Commit — e.g. `chore(release): bump pnpm/action-setup` for a + # dependency update. A human-written `chore(release):` commit + # filtered out here would never trigger a release for its own + # changes, AND would be filtered out of the auto-changeset summary + # on the next run (see scripts/auto-changeset.ts) — silently + # invisible in release notes. The bot identity is set only by + # this workflow's Configure git identity step, so combining the + # two prevents human commits from accidentally matching. + if: ${{ github.event_name == 'workflow_dispatch' || !(startsWith(github.event.head_commit.message, 'chore(release):') && github.event.head_commit.committer.name == 'have-release-bot') }} steps: # Mint a token from the HAVE_RELEASE GitHub App (org-wide secret # pair). Used as GITHUB_TOKEN (for the chore(release) push back to @@ -323,16 +334,24 @@ jobs: # marker for "commit was created by the auto release path". if ! printf '%s' "$HEAD_MSG" | grep -q '^chore(release): bump ' \ || [ "$HEAD_COMMITTER" != "have-release-bot" ]; then - # Escape `%`, CR, LF before embedding user-controlled commit - # subject in a workflow command — per GitHub's + # Escape `%`, CR, LF before embedding user-controlled + # git metadata in workflow commands — per GitHub's # workflow-commands docs, those bytes can corrupt the - # command payload or inject additional commands. - HEAD_MSG_ESCAPED="${HEAD_MSG//%/%25}" - HEAD_MSG_ESCAPED="${HEAD_MSG_ESCAPED//$'\r'/%0D}" - HEAD_MSG_ESCAPED="${HEAD_MSG_ESCAPED//$'\n'/%0A}" + # command payload or inject additional commands. Git + # accepts CR/LF in committer names (verified empirically), + # so both HEAD_MSG and HEAD_COMMITTER need escaping. + escape_wc() { + local s="$1" + s="${s//%/%25}" + s="${s//$'\r'/%0D}" + s="${s//$'\n'/%0A}" + printf '%s' "$s" + } + HEAD_MSG_ESCAPED=$(escape_wc "$HEAD_MSG") + HEAD_COMMITTER_ESCAPED=$(escape_wc "$HEAD_COMMITTER") echo "::error::Recovery requires HEAD to be a 'chore(release): bump …' commit by have-release-bot." echo "::error::HEAD subject: $HEAD_MSG_ESCAPED" - echo "::error::HEAD committer: $HEAD_COMMITTER" + echo "::error::HEAD committer: $HEAD_COMMITTER_ESCAPED" echo "::error::If the failed release commit is no longer at HEAD, resolve manually — do not use this recovery path." exit 1 fi diff --git a/scripts/auto-changeset.ts b/scripts/auto-changeset.ts index 6a2989d..b98c267 100644 --- a/scripts/auto-changeset.ts +++ b/scripts/auto-changeset.ts @@ -41,14 +41,28 @@ const PACKAGES = [ interface Commit { hash: string; + committer: string; subject: string; } -// Subject-only output: hash + space + subject + newline per commit. -// We don't fetch the body (we don't scan footers — see top-of-file -// JSDoc), so newlines are safe as record separators: git commit -// subjects can't contain newlines. -const GIT_PRETTY_FORMAT = `%H %s`; +// Three fields per line: hash, committer name, subject. Records are +// separated by newline (subjects can't contain newlines). Fields are +// separated by ASCII 31 (Unit Separator) emitted via git's `%x1f` +// pretty-format escape — neither committer names nor subjects contain +// that byte in practice, and emitting it via `%x` keeps the byte +// OUT of argv (Node rejects NUL in argv and is awkward with other +// control bytes). +// +// We need committer name to distinguish workflow-generated +// `chore(release): bump …` commits (always committed by +// `have-release-bot`) from identically-prefixed human commits like +// `chore(release): bump pnpm/action-setup` — a valid Conventional +// Commit that a human might write for a dependency bump. Filtering +// by subject alone would silently exclude that human commit from +// the auto-generated changelog. +const GIT_PRETTY_FORMAT = `%H%x1f%cn%x1f%s`; +const FIELD_SEP = '\x1f'; +const RELEASE_BOT_NAME = 'have-release-bot'; /** * Run a git subcommand with args passed positionally. Uses execFileSync @@ -110,19 +124,15 @@ function getCommitsSinceLastRelease(): Commit[] { const range = lastTag ? `${lastTag}..HEAD` : 'HEAD'; const log = git('log', range, `--pretty=format:${GIT_PRETTY_FORMAT}`, '--no-merges'); if (!log) return []; - // Split on newlines — git subjects can't contain newlines, so - // there's no ambiguity. First space splits hash from subject. + // Split on newlines (subjects can't contain newlines), then split + // each line on the field separator into hash, committer, subject. return log .split('\n') .map((line) => line.trim()) .filter(Boolean) .map((line) => { - const spaceIdx = line.indexOf(' '); - if (spaceIdx === -1) return { hash: line, subject: '' }; - return { - hash: line.slice(0, spaceIdx), - subject: line.slice(spaceIdx + 1), - }; + const [hash = '', committer = '', subject = ''] = line.split(FIELD_SEP); + return { hash, committer, subject }; }); } @@ -152,7 +162,18 @@ function main(): void { } const all = getCommitsSinceLastRelease(); - const real = all.filter((c) => !c.subject.startsWith('chore(release)')); + // Filter out workflow-created release commits — but only when BOTH + // subject and committer match the bot's signature. A human commit + // like `chore(release): bump pnpm/action-setup` (valid Conventional + // Commit for a dependency update) would otherwise be silently + // excluded from the changelog and the changes it represents would + // never appear in release notes. + const real = all.filter( + (c) => !( + c.subject.startsWith('chore(release)') && + c.committer === RELEASE_BOT_NAME + ), + ); if (real.length === 0) { console.log( From 1c1bf84ec08d3db874861ffc34a9c86fd68a8248 Mon Sep 17 00:00:00 2001 From: Will Griffin Date: Fri, 22 May 2026 22:25:30 -0600 Subject: [PATCH 16/16] =?UTF-8?q?fix(release):=20round-15=20=E2=80=94=20re?= =?UTF-8?q?place=20awk=20early-exit=20with=20full-scan=20to=20avoid=20SIGP?= =?UTF-8?q?IPE=20bypass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-15 ensemble: codex-cli clean, copilot-cli caught a real high-confidence bug. The step-0 untagged-release detector used `awk '/.../ {print $1; exit}'` to find the first match efficiently. Under `set -o pipefail`, that early exit closes awk's stdin, git log dies on SIGPIPE (exit 141), and the pipeline returns non-zero. Bash's default behavior (no `inherit_errexit`) is that command substitution swallows pipeline failures silently. So `UNTAGGED_RELEASE=$(... | awk ... exit)` produces empty output when the pipeline fails — the subsequent `if [ -n "$UNTAGGED_RELEASE" ]` check sees empty, skips the abort, and the detector is bypassed ENTIRELY. The release proceeds against a state that contains an untagged release commit, producing the exact duplicate-changelog scenario this detector was added to prevent. Fix: full-scan awk with deferred print: ```awk /^[a-f0-9]+ chore\(release\): bump / && !found {hash=$1; found=1} END {if (found) print hash} ``` Same "first match wins" semantics, no early exit, no SIGPIPE. Verified on three input cases (match, no-match, empty) under `set -euo pipefail` — all return cleanly without tripping the pipeline failure path. Not applying copilot's other two findings: - Legacy `chore(release): v` format support: this repo has no prior releases; no legacy state exists. The detector only needs to recognize commits this workflow itself creates. - Stronger committer identity (name + email): real concern but the threat model is "anyone with push access". Push access already permits many forms of release pipeline interference. Adding email check is marginal defense in depth; keeping name-only for now. --- .github/workflows/release.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8c1562c..3e70f3e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -145,10 +145,21 @@ jobs: # `git log --grep` would match commit bodies (subject + # body), not just subjects, so we use `--committer` for # identity but pipe `%s` through awk for subject matching. + # + # awk scans the full input (no `exit` on match) deliberately: + # under `set -o pipefail`, an early `exit` would close awk's + # stdin, git log would die on SIGPIPE (exit 141), and the + # pipeline would return non-zero. Bash command substitution + # without `inherit_errexit` silently swallows the failure + # and produces empty $UNTAGGED_RELEASE — the detector would + # be bypassed entirely. Letting awk consume all input + # avoids SIGPIPE. Recording the first match in a variable + # and printing at END preserves "first match wins" semantics + # without the early-exit hazard. UNTAGGED_RELEASE=$(git log "$UNTAGGED_RANGE" \ --committer='have-release-bot' \ --pretty='%H %s' \ - | awk '/^[a-f0-9]+ chore\(release\): bump / {print $1; exit}') + | awk '/^[a-f0-9]+ chore\(release\): bump / && !found {hash=$1; found=1} END {if (found) print hash}') if [ -n "$UNTAGGED_RELEASE" ]; then echo "::error::Found untagged chore(release): bump commit $UNTAGGED_RELEASE in range $UNTAGGED_RANGE." echo "::error::The previous release left an unfinished state — either changeset:publish or the tag push failed before completing."