Skip to content

fix(ci): give internal persona publishes one reconcilable release commit - #328

Merged
khaliqgant merged 2 commits into
mainfrom
fix/internal-personas-release-push
Aug 24, 2026
Merged

fix(ci): give internal persona publishes one reconcilable release commit#328
khaliqgant merged 2 commits into
mainfrom
fix/internal-personas-release-push

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Aug 24, 2026

Copy link
Copy Markdown
Member

Why

publish-internal-personas.yml is 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 with git push origin HEAD --follow-tags.

That has both failure modes #326 fixed, and a third:

  • a branch that moves mid-run leaves a chain of release commits that can't be rebuilt onto the tip (the reconcile script models a single commit)
  • tags are created before the push, so a rejection strands them
  • the checkout is the pinned workflow_dispatch SHA, so a queued run bumps from a stale base

Change

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 takes github.ref_name.

The per-pack git commit became a git add — the index carries across steps on the same runner, so the release step just commits what the loop staged.

Tests

  • The structural pass now covers all three publishing workflows, and asserts none of them still contains git push origin HEAD --follow-tags.
  • New behavioral test runs the extracted Commit + push release step 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

Review in cubic

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>
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 57 minutes.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 16faac49-1c58-4dea-a371-06a39082f934

📥 Commits

Reviewing files that changed from the base of the PR and between 2e1f628 and 6e327e1.

📒 Files selected for processing (5)
  • .github/workflows/publish-internal-personas.yml
  • .github/workflows/publish-persona.yml
  • .github/workflows/publish.yml
  • scripts/push-release-commit.sh
  • scripts/release-workflows.test.mjs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +243 to +244
env:
BRANCH: ${{ github.ref_name }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread .github/workflows/publish-internal-personas.yml
* 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-'));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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."

@khaliqgant
khaliqgant merged commit b9df745 into main Aug 24, 2026
3 checks passed
@khaliqgant
khaliqgant deleted the fix/internal-personas-release-push branch August 24, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant