fix(ci): give internal persona publishes one reconcilable release commit - #328
Conversation
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 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 57 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbb7f2744f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| env: | ||
| BRANCH: ${{ github.ref_name }} |
There was a problem hiding this comment.
Reject tag refs before mapping them to a branch
When this workflow is dispatched with a tag ref, github.ref_name is the tag name, but push-release-commit.sh always pushes HEAD to refs/heads/$BRANCH; the newly published versions therefore create/update a branch named after the tag instead of reconciling onto an intended release branch, leaving that branch's manifests stale after npm has already changed. This is a supported invocation—gh workflow run --help describes --ref as a “Branch or tag name”—so the inspected workflow should reject tag dispatches before publishing or require an explicit target branch rather than passing the tag name as BRANCH.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/release-workflows.test.mjs">
<violation number="1" location="scripts/release-workflows.test.mjs:302">
P3: The test creates a temp repo via `mkdtempSync` and writes the fabricated TSV to the global `/tmp/persona-publish-targets.tsv`, but never removes either, leaving them behind after every run (and the fixed /tmp path can collide if a stale copy from an aborted run lingers). Wrap the body in try/finally and `rmSync(root, { recursive: true, force: true })` (plus the /tmp TSV) after the assertions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * 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-')); |
There was a problem hiding this comment.
P3: The test creates a temp repo via mkdtempSync and writes the fabricated TSV to the global /tmp/persona-publish-targets.tsv, but never removes either, leaving them behind after every run (and the fixed /tmp path can collide if a stale copy from an aborted run lingers). Wrap the body in try/finally and rmSync(root, { recursive: true, force: true }) (plus the /tmp TSV) after the assertions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/release-workflows.test.mjs, line 302:
<comment>The test creates a temp repo via `mkdtempSync` and writes the fabricated TSV to the global `/tmp/persona-publish-targets.tsv`, but never removes either, leaving them behind after every run (and the fixed /tmp path can collide if a stale copy from an aborted run lingers). Wrap the body in try/finally and `rmSync(root, { recursive: true, force: true })` (plus the /tmp TSV) after the assertions.</comment>
<file context>
@@ -267,3 +291,44 @@ for (const [name, workflow] of [
+ * 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);
+
</file context>
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/<tag> 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 <noreply@anthropic.com>
There was a problem hiding this comment.
1 existing issue remains and 1 new issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/publish.yml">
<violation number="1" location=".github/workflows/publish.yml:64">
P2: A branch name containing shell syntax such as `$(...)` executes during this diagnostic because GitHub interpolates `github.ref_name` into the Bash script. Read `GITHUB_REF_NAME` and `GITHUB_REF_TYPE` from the environment instead of embedding the context values in `run`.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
| - 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." |
There was a problem hiding this comment.
P2: A branch name containing shell syntax such as $(...) executes during this diagnostic because GitHub interpolates github.ref_name into the Bash script. Read GITHUB_REF_NAME and GITHUB_REF_TYPE from the environment instead of embedding the context values in run.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish.yml, line 64:
<comment>A branch name containing shell syntax such as `$(...)` executes during this diagnostic because GitHub interpolates `github.ref_name` into the Bash script. Read `GITHUB_REF_NAME` and `GITHUB_REF_TYPE` from the environment instead of embedding the context values in `run`.</comment>
<file context>
@@ -55,6 +55,15 @@ jobs:
+ - 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
+
</file context>
| 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." | |
| 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." |
Why
publish-internal-personas.ymlis the last workflow still carrying the shape that broke on 2026-08-24. It committed and tagged inside its publish loop — one commit per pack, interleaved with the npm publishes — then pushed the chain at the end withgit push origin HEAD --follow-tags.That has both failure modes #326 fixed, and a third:
workflow_dispatchSHA, so a queued run bumps from a stale baseChange
Stage every bump in the loop, make one release commit after it, push through
scripts/push-release-commit.sh, and create tags only once that commit is on the branch. Checkout takesgithub.ref_name.The per-pack
git commitbecame agit add— the index carries across steps on the same runner, so the release step just commits what the loop staged.Tests
git push origin HEAD --follow-tags.Commit + push releasestep against a staged index with two packs, asserting exactly one release commit naming both, and that the push is delegated to the shared script.Both fail against the per-pack commit shape — verified by restoring it and re-running.
Note
This workflow publishes each pack to npm inside the loop before any of them is committed, which was already true. The reconcile only makes the git side recoverable; a mid-loop npm failure still leaves earlier packs published, same as before.
🤖 Generated with Claude Code