From fbb7f2744f58905a58e08eae2e4c0865f5fda56f Mon Sep 17 00:00:00 2001 From: Ricky Schema Cascade Date: Mon, 24 Aug 2026 14:46:28 +0200 Subject: [PATCH 1/2] fix(ci): give internal persona publishes one reconcilable release commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last workflow still carrying the 2026-08-24 failure shape. It committed and tagged inside its publish loop — one commit per pack, interleaved with the npm publishes — so a branch that moved mid-run left it with a chain of commits that cannot be rebuilt on the tip, and tags already created for a push that then gets rejected. Stage every bump in the loop, make one release commit after it, push it through scripts/push-release-commit.sh, and create the tags only once that commit is on the branch. Also checks out the branch tip rather than the pinned dispatch SHA, like the other two. Tests: the structural pass now covers all three publishing workflows and asserts none of them still uses `git push origin HEAD --follow-tags`; a new behavioral test runs the commit step against a staged index and asserts one release commit naming every pack, with the push delegated. Both fail against the per-pack commit shape. Co-Authored-By: Claude Opus 5 --- .../workflows/publish-internal-personas.yml | 56 +++++++++++-- scripts/release-workflows.test.mjs | 83 +++++++++++++++++-- 2 files changed, 123 insertions(+), 16 deletions(-) diff --git a/.github/workflows/publish-internal-personas.yml b/.github/workflows/publish-internal-personas.yml index 4ee7a463..e4af40e0 100644 --- a/.github/workflows/publish-internal-personas.yml +++ b/.github/workflows/publish-internal-personas.yml @@ -66,6 +66,10 @@ jobs: uses: actions/checkout@v6 with: fetch-depth: 0 + # `workflow_dispatch` pins `github.sha` at dispatch time; a queued run + # would otherwise bump from a stale commit. See the same note in + # publish.yml. + ref: ${{ github.ref_name }} - name: Setup pnpm uses: pnpm/action-setup@v5 @@ -219,19 +223,57 @@ jobs: echo "==> Publishing $TARBALL $COMMON_FLAGS" npm publish "$TARBALL" $COMMON_FLAGS + # Staged, not committed: the run makes ONE release commit after the + # loop. A commit per persona cannot be reconciled onto a branch that + # moved mid-run, and tagging here would strand tags whenever the + # push is rejected — see publish.yml and scripts/push-release-commit.sh. if [ "$INPUT_DRY_RUN" != "true" ] && [ "$INPUT_VERSION" != "none" ]; then git add "$DIR/package.json" - if ! git diff --cached --quiet; then - git commit -m "chore(release): $NAME@$VERSION" - fi - SLUG="$(echo "${NAME#@}" | tr '/' '-')" - git tag -a "${SLUG}-v${VERSION}" -m "$NAME@$VERSION" fi echo "$NAME@$VERSION published (dry_run=$INPUT_DRY_RUN)" >> "$GITHUB_STEP_SUMMARY" echo "::endgroup::" done < /tmp/persona-publish-targets.tsv - - name: Push commits + tags + # The packs are on npm by the time this runs, so the push reconciles onto + # whatever landed on the branch mid-run rather than failing and leaving + # the registry ahead of git. + - name: Commit + push release if: ${{ github.event.inputs.dry_run != 'true' && github.event.inputs.version != 'none' }} - run: git push origin HEAD --follow-tags + env: + BRANCH: ${{ github.ref_name }} + run: | + set -euo pipefail + if git diff --cached --quiet; then + echo "No version changes to commit." + exit 0 + fi + + MSG="chore(release):" + while IFS=$'\t' read -r NAME DIR VERSION; do + MSG="$MSG $NAME@$VERSION" + done < /tmp/persona-publish-targets.tsv + git commit -m "$MSG" + + scripts/push-release-commit.sh + + - name: Tag + push tags + if: ${{ github.event.inputs.dry_run != 'true' && github.event.inputs.version != 'none' }} + run: | + set -euo pipefail + CREATED="" + while IFS=$'\t' read -r NAME DIR VERSION; do + SLUG="$(echo "${NAME#@}" | tr '/' '-')" + TAG="${SLUG}-v${VERSION}" + if git rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then + echo "::warning::tag $TAG already exists - leaving it as is" + continue + fi + git tag -a "$TAG" -m "$NAME@$VERSION" + CREATED="$CREATED refs/tags/$TAG" + done < /tmp/persona-publish-targets.tsv + if [ -n "$CREATED" ]; then + set -f + git push origin $CREATED + set +f + fi diff --git a/scripts/release-workflows.test.mjs b/scripts/release-workflows.test.mjs index 3cc7cff9..958910c5 100644 --- a/scripts/release-workflows.test.mjs +++ b/scripts/release-workflows.test.mjs @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import { execFileSync } from 'node:child_process'; -import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import test from 'node:test'; @@ -8,6 +8,10 @@ import test from 'node:test'; const publishWorkflow = readFileSync('.github/workflows/publish.yml', 'utf8'); const verifyWorkflow = readFileSync('.github/workflows/verify-publish.yml', 'utf8'); const personaWorkflow = readFileSync('.github/workflows/publish-persona.yml', 'utf8'); +const internalPersonaWorkflow = readFileSync( + '.github/workflows/publish-internal-personas.yml', + 'utf8' +); function publishTargetDirectories(workflow) { const match = workflow.match(/echo "packages=([^"]+)"/); @@ -85,6 +89,21 @@ test('scoped CLI verification checks only the supported thin-entry contract', () */ const pushScript = 'scripts/push-release-commit.sh'; +function stepScript(workflow, name) { + const lines = workflow.replaceAll('\r\n', '\n').split('\n'); + const start = lines.findIndex((line) => line.trim() === `- name: ${name}`); + assert.notEqual(start, -1, `workflow must define a "${name}" step`); + + const next = lines.findIndex((line, index) => index > start && /^\s*- name: /.test(line)); + const stepLines = lines.slice(start, next === -1 ? lines.length : next); + const runIndex = stepLines.findIndex((line) => /^\s+run: \|\s*$/.test(line)); + assert.notEqual(runIndex, -1, `"${name}" must carry a literal run block`); + + const body = stepLines.slice(runIndex + 1); + const indent = body.find((line) => line.trim())?.match(/^\s*/)[0] ?? ''; + return body.map((line) => (line.startsWith(indent) ? line.slice(indent.length) : line)).join('\n'); +} + const GIT_ENV = { ...process.env, GIT_AUTHOR_NAME: 'release-test', @@ -242,20 +261,25 @@ test('release commit is a no-op when the branch already carries its files', () = assert.equal(versionOnMain(seed, 'cli'), '4.1.49'); }); -for (const [name, workflow] of [ - ['publish.yml', publishWorkflow], - ['publish-persona.yml', personaWorkflow], +// Every workflow that publishes to npm and then updates git. `push` names the +// step that lands the release commit; each must reconcile, and must tag only +// after that commit is on the branch. +for (const [name, workflow, push] of [ + ['publish.yml', publishWorkflow, 'Push release commit'], + ['publish-persona.yml', personaWorkflow, 'Push release commit'], + ['publish-internal-personas.yml', internalPersonaWorkflow, 'Commit + push release'], ]) { test(`${name} pushes the release commit before tagging it`, () => { const lines = workflow.split('\n'); - const push = lines.findIndex((line) => line.trim() === '- name: Push release commit'); + const pushStep = lines.findIndex((line) => line.trim() === `- name: ${push}`); const tag = lines.findIndex((line) => line.trim() === '- name: Tag + push tags'); - assert.notEqual(push, -1, 'must reconcile its push'); + assert.notEqual(pushStep, -1, 'must reconcile its push'); assert.notEqual(tag, -1, 'must tag in its own step'); - assert.ok(push < tag, 'tagging before the push can strand tags on an unreachable commit'); + assert.ok(pushStep < tag, 'tagging before the push can strand tags on an unreachable commit'); + assert.ok(workflow.includes(pushScript), 'must use the shared reconciling push script'); assert.ok( - workflow.includes(`run: ${pushScript}`), - 'must use the shared reconciling push script' + !/git push origin HEAD --follow-tags/.test(workflow), + 'the unreconciled push is what left npm ahead of git on 2026-08-24' ); }); @@ -267,3 +291,44 @@ for (const [name, workflow] of [ ); }); } + +/** + * publish-internal-personas.yml used to commit and tag once per persona inside + * its publish loop, which cannot be reconciled onto a branch that moved. It now + * stages every bump and makes one release commit after the loop; this exercises + * that step against a staged index rather than trusting the YAML to read right. + */ +test('internal personas make a single release commit for every pack', () => { + const root = mkdtempSync(join(tmpdir(), 'persona-release-')); + git(root, 'init', '-q', '-b', 'main', root); + + for (const [dir, version] of [['persona-a', '1.2.3'], ['persona-b', '4.5.6']]) { + mkdirSync(join(root, 'packages', dir), { recursive: true }); + writeFileSync(join(root, 'packages', dir, 'package.json'), `{"version":"${version}"}\n`); + } + git(root, 'add', '-A'); + git(root, 'commit', '-qm', 'base'); + + // What the publish loop leaves behind: bumped manifests, staged, uncommitted. + writeFileSync(join(root, 'packages', 'persona-a', 'package.json'), '{"version":"1.2.4"}\n'); + writeFileSync(join(root, 'packages', 'persona-b', 'package.json'), '{"version":"4.5.7"}\n'); + git(root, 'add', '-A'); + writeFileSync( + '/tmp/persona-publish-targets.tsv', + '@scope/persona-a\tpackages/persona-a\t1.2.4\n@scope/persona-b\tpackages/persona-b\t4.5.7\n' + ); + + // The step ends by delegating the push; stub it so the test stays local. + mkdirSync(join(root, 'scripts'), { recursive: true }); + writeFileSync(join(root, 'scripts', 'push-release-commit.sh'), '#!/bin/sh\ntouch pushed.marker\n'); + execFileSync('chmod', ['+x', join(root, 'scripts', 'push-release-commit.sh')]); + + const script = join(root, 'commit-step.sh'); + writeFileSync(script, stepScript(internalPersonaWorkflow, 'Commit + push release')); + execFileSync('/bin/bash', [script], { cwd: root, encoding: 'utf8', env: GIT_ENV }); + + const subjects = git(root, 'log', '--format=%s').trim().split('\n'); + assert.equal(subjects.length, 2, 'one release commit on top of the base, not one per pack'); + assert.equal(subjects[0], 'chore(release): @scope/persona-a@1.2.4 @scope/persona-b@4.5.7'); + assert.ok(readdirSync(root).includes('pushed.marker'), 'must delegate to the push script'); +}); From 6e327e1278841d6b8363355a09c6073a3f226c71 Mon Sep 17 00:00:00 2001 From: Ricky Schema Cascade Date: Mon, 24 Aug 2026 15:00:39 +0200 Subject: [PATCH 2/2] fix(ci): refuse a publish dispatched from a tag (PR feedback) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cubic (P1) and codex both caught it: the workflows pass `github.ref_name` as the branch to push to, which on a tag dispatch is the tag name. The release commit would land on a newly created refs/heads/ while the real branch stayed stale — npm ahead of git again, by a different route. Guarded in two places, because by the time the push runs the packages are already published: - Each of the three workflows fails on `github.ref_type != 'branch'` as its first step, before anything is built or published. - push-release-commit.sh refuses a target that is not an existing branch on origin, as the backstop if a caller passes something else. Also cleans up the temp repos and the fixed /tmp fixture path the tests were leaving behind (cubic P3). Tests: every publishing workflow must carry the guard as its first step, and the script must refuse a tag target without creating a branch for it. Both fail with their guard removed. Co-Authored-By: Claude Opus 5 --- .../workflows/publish-internal-personas.yml | 9 ++++ .github/workflows/publish-persona.yml | 9 ++++ .github/workflows/publish.yml | 9 ++++ scripts/push-release-commit.sh | 9 ++++ scripts/release-workflows.test.mjs | 41 ++++++++++++++++++- 5 files changed, 76 insertions(+), 1 deletion(-) diff --git a/.github/workflows/publish-internal-personas.yml b/.github/workflows/publish-internal-personas.yml index e4af40e0..0a21c552 100644 --- a/.github/workflows/publish-internal-personas.yml +++ b/.github/workflows/publish-internal-personas.yml @@ -62,6 +62,15 @@ jobs: name: Publish persona packs runs-on: ubuntu-latest steps: + # `github.ref_name` is the branch this run pushes its release commit to. + # On a tag dispatch it would be the tag name, and the push would create a + # branch named after the tag while npm already has the new versions. + - name: Require a branch dispatch + if: ${{ github.ref_type != 'branch' }} + run: | + echo "::error title=Dispatch from a branch::This workflow publishes and then pushes a release commit to '${{ github.ref_name }}', which is a ${{ github.ref_type }}. Re-run it from a branch." + exit 1 + - name: Checkout uses: actions/checkout@v6 with: diff --git a/.github/workflows/publish-persona.yml b/.github/workflows/publish-persona.yml index a544b1e5..b0285d01 100644 --- a/.github/workflows/publish-persona.yml +++ b/.github/workflows/publish-persona.yml @@ -61,6 +61,15 @@ jobs: npm_name: ${{ steps.package.outputs.npm_name }} tag_name: personas-core-v${{ steps.bump.outputs.version }} steps: + # `github.ref_name` is the branch this run pushes its release commit to. + # On a tag dispatch it would be the tag name, and the push would create a + # branch named after the tag while npm already has the new versions. + - name: Require a branch dispatch + if: ${{ github.ref_type != 'branch' }} + run: | + echo "::error title=Dispatch from a branch::This workflow publishes and then pushes a release commit to '${{ github.ref_name }}', which is a ${{ github.ref_type }}. Re-run it from a branch." + exit 1 + - name: Checkout uses: actions/checkout@v6 with: diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 94ed7a9d..b6d79b26 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -55,6 +55,15 @@ jobs: versions: ${{ steps.bump.outputs.versions }} release_version: ${{ steps.bump.outputs.release_version }} steps: + # `github.ref_name` is the branch this run pushes its release commit to. + # On a tag dispatch it would be the tag name, and the push would create a + # branch named after the tag while npm already has the new versions. + - name: Require a branch dispatch + if: ${{ github.ref_type != 'branch' }} + run: | + echo "::error title=Dispatch from a branch::This workflow publishes and then pushes a release commit to '${{ github.ref_name }}', which is a ${{ github.ref_type }}. Re-run it from a branch." + exit 1 + - name: Checkout uses: actions/checkout@v6 with: diff --git a/scripts/push-release-commit.sh b/scripts/push-release-commit.sh index a431d940..feed12e1 100755 --- a/scripts/push-release-commit.sh +++ b/scripts/push-release-commit.sh @@ -23,6 +23,15 @@ set -euo pipefail BRANCH="${BRANCH:-main}" ATTEMPTS="${PUSH_ATTEMPTS:-5}" +# Callers pass `github.ref_name`, which is a tag name on a tag dispatch. Pushing +# HEAD to refs/heads/ would invent a branch named after the tag and leave +# the real branch stale while npm already has the new versions. The workflows +# reject a non-branch dispatch before publishing; this is the backstop. +if ! git show-ref --verify --quiet "refs/remotes/origin/$BRANCH"; then + echo "::error title=Release commit not pushed::'$BRANCH' is not an existing branch on origin. Publish from a branch." >&2 + exit 1 +fi + for attempt in $(seq 1 "$ATTEMPTS"); do if git push origin "HEAD:refs/heads/$BRANCH"; then echo "Pushed the release commit to $BRANCH on attempt $attempt." diff --git a/scripts/release-workflows.test.mjs b/scripts/release-workflows.test.mjs index 958910c5..8f0e1ced 100644 --- a/scripts/release-workflows.test.mjs +++ b/scripts/release-workflows.test.mjs @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import { execFileSync } from 'node:child_process'; -import { mkdirSync, mkdtempSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import test from 'node:test'; @@ -116,6 +116,11 @@ function git(cwd, ...args) { return execFileSync('git', args, { cwd, encoding: 'utf8', env: GIT_ENV }); } +/** Temp repos are throwaway, but leaving a pile of them in tmpdir is rude. */ +function cleanup(...paths) { + for (const path of paths) rmSync(path, { recursive: true, force: true }); +} + function writeVersions(dir, version) { for (const pkg of ['cli', 'deploy']) { mkdirSync(join(dir, 'packages', pkg), { recursive: true }); @@ -246,6 +251,29 @@ test('exhausted attempts fail loudly instead of stranding a rebuilt commit', () assert.equal(git(run, 'rev-parse', 'HEAD').trim(), releaseBefore); }); +test('a non-branch target is refused before anything is pushed', () => { + const { root, seed, run } = stageRelease(); + try { + // What a tag dispatch produces: github.ref_name is the tag, not a branch. + git(seed, 'tag', '-a', 'v9.9.9', '-m', 'a tag'); + git(seed, 'push', '-q', 'origin', 'refs/tags/v9.9.9'); + git(run, 'fetch', '-q', 'origin'); + + assert.throws( + () => runPushStep(run, { BRANCH: 'v9.9.9' }), + /not an existing branch on origin/, + 'pushing HEAD to refs/heads/ would invent a branch named after the tag' + ); + git(seed, 'fetch', '-q', 'origin'); + assert.throws( + () => git(seed, 'rev-parse', '--verify', 'origin/v9.9.9'), + 'no branch may be created for the tag' + ); + } finally { + cleanup(root); + } +}); + test('release commit is a no-op when the branch already carries its files', () => { const { seed, run } = stageRelease(); @@ -283,6 +311,15 @@ for (const [name, workflow, push] of [ ); }); + test(`${name} refuses a dispatch that is not from a branch`, () => { + const lines = workflow.split('\n'); + const guard = lines.findIndex((line) => line.trim() === '- name: Require a branch dispatch'); + assert.notEqual(guard, -1, 'a tag dispatch would push the release commit to refs/heads/'); + const firstStep = lines.findIndex((line) => /^\s*- name: /.test(line)); + assert.equal(guard, firstStep, 'the guard must run before anything is published'); + assert.match(workflow, /if: \$\{\{ github\.ref_type != 'branch' \}\}/); + }); + test(`${name} checks out the branch tip, not the dispatch SHA`, () => { assert.match( workflow, @@ -331,4 +368,6 @@ test('internal personas make a single release commit for every pack', () => { assert.equal(subjects.length, 2, 'one release commit on top of the base, not one per pack'); assert.equal(subjects[0], 'chore(release): @scope/persona-a@1.2.4 @scope/persona-b@4.5.7'); assert.ok(readdirSync(root).includes('pushed.marker'), 'must delegate to the push script'); + + cleanup(root, '/tmp/persona-publish-targets.tsv'); });