Skip to content

prevent creating a release when the pinned js sdk version is differen… #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
franzwarning merged 8 commits into from
Aug 12, 2026
Merged

prevent creating a release when the pinned js sdk version is differen… #53

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
931208d
prevent creating a release when the pinned js sdk version is differen…
The64thRealm Aug 11, 2026
d9e029e
use a fine-grained PAT instead of a GitHub App to read play
franzwarning Aug 11, 2026
a0a50cf
drop the CLAUDE.md changes
franzwarning Aug 11, 2026
1d410df
only run the gate when the push would actually cut a new tag, slim th…
franzwarning Aug 11, 2026
e2999ba
restore CLAUDE.md to main, no changes to it in this PR
franzwarning Aug 11, 2026
a214795
re-trigger the gate on sdk pin bumps so the documented retry works
franzwarning Aug 11, 2026
2c24236
remove CLAUDE.md
franzwarning Aug 11, 2026
8d5e6bb
remove workflow_dispatch
franzwarning Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions .github/workflows/auto-tag.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
name: Auto Tag on Version Change

on:
workflow_dispatch:
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Outdated

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

workflow_dispatch with no branch guard lets this job cut a release tag from any branch. Dispatching the workflow with "Use workflow from branch: some-feature-branch" checks out that branch, reads its Cargo.toml version, and — if that version isn't tagged yet — pushes the tag, which fires release.yml (tag pattern **[0-9]+.[0-9]+.[0-9]+*) and publishes binaries plus the Homebrew formula from non-main code. Before this change only pushes to main could produce a tag. Worth pinning the job to main:

Suggested change
workflow_dispatch:
workflow_dispatch:

plus a job-level guard, e.g. if: github.ref == 'refs/heads/main'.

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

workflow_dispatch with no branch guard turns "cut a release" into a button anyone with write access can press on any branch. (Raised in the previous round and by Devin; a214795 added the trigger without the guard, so re-flagging once.)

For workflow_dispatch, actions/checkout@v4 checks out the ref picked in the UI. The job then reads version from that branch's Cargo.toml (line 27), finds no matching remote tag, and pushes an annotated tag at that branch's HEAD (line 63-64) using HOMEBREW_TAP_TOKEN. release.yml fires on version-shaped tag pushes and publishes binaries plus the Homebrew formula. Before a214795, only a push to main could produce a tag.

The sdk gate is no help here — it reads the pin from the same unreviewed branch, so a branch that bumps both files sails through.

Concretely: open a branch, set version = "9.9.9", dispatch Auto Tag on it → 9.9.9 is tagged at unreviewed code and shipped to the tap.

A job-level guard keeps the manual retry path (dispatch on main) while restoring the main-only invariant:

jobs:
  tag:
    runs-on: ubuntu-latest
    if: github.ref == 'refs/heads/main'

Worth deciding whether you want workflow_dispatch at all now that src/dev/sdk-js-version is in paths — the pin-only push covers the documented retry on its own, and dropping the trigger closes this without a guard.

push:
branches:
- main
paths:
- 'Cargo.toml'
- 'src/dev/sdk-js-version'

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Adding this path is the right fix for the retry, but it also widens the check-then-push race that 1d410df opened, and there's still no concurrency: key on the workflow.

Two runs can now be in flight for the same Cargo.toml version, which wasn't possible before this line:

  1. A version bump 0.1.92 → 0.1.93 merges. Run 1: ls-remote says untagged → gate runs (two gh api round-trips to play, several seconds).
  2. A pin bump merges 20s later — which this line now makes a trigger. Run 2 reads the same 0.1.93, ls-remote still says untagged (run 1 hasn't pushed yet) → gate runs.
  3. Run 1 pushes 0.1.93. Run 2 reaches line 64 and dies on rejected — already exists.

That used to be a green no-op twice over: the pin push queued no run at all, and || echo "Tag already pushed" absorbed the collision. Now it's a red X and a failure notification on main for a commit that was correct.

Nothing double-releases, so this is noise rather than danger — but it's the same class of noise 1d410df set out to remove, and it's one key at the top of the file:

concurrency:
  group: auto-tag
  cancel-in-progress: false

That serialises the runs so the second one's ls-remote observes the first one's tag and skips cleanly.


jobs:
tag:
Expand All @@ -16,7 +18,7 @@ jobs:
- uses: actions/checkout@v4
with:
token: ${{ secrets.HOMEBREW_TAP_TOKEN }}

- name: Get version
Comment thread
franzwarning marked this conversation as resolved.
id: get_version
run: |
Expand All @@ -26,10 +28,37 @@ jobs:
echo "Detected version: $VERSION"
echo "version=$VERSION" >> $GITHUB_OUTPUT
Comment thread
franzwarning marked this conversation as resolved.

- name: Check if version is already tagged
id: tagged
run: |
if git ls-remote --exit-code origin "refs/tags/${{ steps.get_version.outputs.version }}" > /dev/null; then
Comment thread
franzwarning marked this conversation as resolved.

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

git ls-remote --exit-code returns 2 for "no matching ref" but 128 for a genuine failure (network blip, expired HOMEBREW_TAP_TOKEN, DNS), and both land in the else branch as exists=false. So an auth/transport failure is reported as "this version isn't tagged yet": the sdk gate runs, and the job then dies at git push origin <tag> with a confusing "tag already exists" rejection instead of surfacing the real cause. Distinguishing the codes would make the failure self-explanatory:

Suggested change
if git ls-remote --exit-code origin "refs/tags/${{ steps.get_version.outputs.version }}" > /dev/null; then
if git ls-remote --exit-code origin "refs/tags/${{ steps.get_version.outputs.version }}" > /dev/null; then
echo "exists=true" >> $GITHUB_OUTPUT
elif [ $? -eq 2 ]; then
echo "exists=false" >> $GITHUB_OUTPUT
else
echo "::error::git ls-remote failed; cannot tell whether ${{ steps.get_version.outputs.version }} is already tagged"
exit 1
fi

echo "exists=true" >> $GITHUB_OUTPUT
else
Comment on lines +31 to +35

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dropping || echo "Tag already exists" made the tag push non-idempotent, and this guard doesn't fully replace it.

Gating on steps.tagged is the right call and it covers the common case. But the old || fallback was also absorbing two cases this if doesn't, and both now surface as a red X on main for a push that would previously have been a silent no-op:

1. ls-remote conflates "no such ref" with "couldn't reach the remote." Per the docs, --exit-code returns 2 when no matching ref is found; a transport or auth failure is a generic fatal, 128. Both are non-zero, so both take the else branch and set exists=false. On a transient blip against an already-tagged version, the job then runs the gate (which can itself hard-fail on a pin that drifted after that version shipped — an error about a release that already happened), creates the tag locally (actions/checkout defaults to fetch-depth: 1 with no tags, so nothing collides), and dies on git push origin <tag> with rejected — already exists.

2. There's no concurrency: key, so two runs can both see exists=false. Merge a version bump, then merge a dependency-only Cargo.toml edit 30 seconds later — both jobs read the same pre-tag state, both pass the guard, and the loser's push is rejected. Narrow, but this workflow's trigger makes back-to-back Cargo.toml pushes normal.

Neither is dangerous — nothing double-releases — but each is a failure notification on a no-op, which is exactly the noise 1d410df set out to remove.

Distinguishing 2 from 128 fixes (1) and makes the intent explicit:

Suggested change
id: tagged
run: |
if git ls-remote --exit-code origin "refs/tags/${{ steps.get_version.outputs.version }}" > /dev/null; then
echo "exists=true" >> $GITHUB_OUTPUT
else
rc=0
git ls-remote --exit-code origin "refs/tags/${{ steps.get_version.outputs.version }}" > /dev/null || rc=$?
if [ "$rc" -eq 0 ]; then
echo "exists=true" >> $GITHUB_OUTPUT
elif [ "$rc" -eq 2 ]; then
echo "exists=false" >> $GITHUB_OUTPUT
else
echo "::error::Could not list tags on origin (git ls-remote exited $rc)"
exit 1
fi

For (2), a top-level concurrency: {group: auto-tag, cancel-in-progress: false} serialises the runs so the second one observes the first one's tag.

echo "exists=false" >> $GITHUB_OUTPUT
fi
Comment thread
franzwarning marked this conversation as resolved.

- name: Verify the pinned sdk-js matches the version play ships
Comment thread
franzwarning marked this conversation as resolved.
if: steps.tagged.outputs.exists == 'false'
env:
GH_TOKEN: ${{ secrets.PLAY_READ_PAT }}
run: |
Comment on lines +42 to +43

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PLAY_READ_PAT is now documented nowhere, and the failure mode it will eventually hit is undiagnosable.

Two things landed together across d9e029e → e2999ba: the ## Releases prose that explained this secret was reverted (correctly — CLAUDE.md should be clean against main), and 1d410df dropped the empty-GH_TOKEN pre-check that d9e029e had added. Net result:

$ grep -rn PLAY_READ .          # excluding .git
./.github/workflows/auto-tag.yml:41

One hit, in the workflow itself. Nothing records that it must be a fine-grained PAT with Contents: read on wvdsh/play, that fine-grained PATs expire (so this will break on a date nobody has written down), or that it's bound to the account that minted it (so it dies when that person's access changes).

The failure is worse than just undocumented, because both variants die before the ::error:: on line 52 can fire:

  • Secret unset or deleted → ${{ secrets.PLAY_READ_PAT }} expands to the empty string, gh treats empty GH_TOKEN as unset, and line 46 dies with gh: To use GitHub CLI in a GitHub Actions workflow, set the GH_TOKEN environment variable. Whoever reads that looks at line 41, sees GH_TOKEN is set, and is stuck.
  • PAT expired → bare gh: Bad credentials (HTTP 401), no annotation, no mention of PLAY_READ_PAT.

Either way every release is hard-blocked, on a step named "Verify the pinned sdk-js matches the version play ships" — which reads like play is ahead, not like the token lapsed.

Restoring the pre-check is three lines and puts the secret's name and requirements in the one place someone will definitely be looking:

Suggested change
GH_TOKEN: ${{ secrets.PLAY_READ_PAT }}
run: |
set -euo pipefail
if [ -z "${GH_TOKEN:-}" ]; then
echo "::error::PLAY_READ_PAT is unset. The release gate needs a fine-grained PAT with Contents:read on wvdsh/play, set as the PLAY_READ_PAT repo secret. Fine-grained PATs expire — if this used to work, reissue it."
exit 1
fi
pinned=$(tr -d '[:space:]' < src/dev/sdk-js-version)

That doesn't cover the 401 case, and it isn't a substitute for writing the runbook down somewhere — but "somewhere" needn't be CLAUDE.md; the secret's own description field or a line in the repo README would do.

set -euo pipefail
pinned=$(tr -d '[:space:]' < src/dev/sdk-js-version)
# play deploys prod on `release: published`, so its latest release is what's live
tag=$(gh api repos/wvdsh/play/releases/latest --jq .tag_name)
Comment thread
franzwarning marked this conversation as resolved.
Comment thread
franzwarning marked this conversation as resolved.
Comment thread
franzwarning marked this conversation as resolved.

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

releases/latest returns the most recent non-prerelease, non-draft release, ordered by the tag's created_at — which doesn't match the comment above it. release: published fires for prereleases too, so if play deploys prod from a prerelease (or from a release whose tag was created earlier than another tag, e.g. a backport cut later), this reads a different release than the one that is live. Concrete failure: play publishes prerelease v2.0.0-rc1 bundling sdk 1.4.0 and prod deploys it; a dev correctly bumps the CLI pin to 1.4.0; this step reads the older full release shipping 1.3.43, blocks the release, and tells the dev to downgrade the pin to a version prod no longer runs.

shipped=$(gh api -H "Accept: application/vnd.github.raw" \
"repos/wvdsh/play/contents/package-lock.json?ref=$tag" \
| jq -er '.packages["node_modules/@wvdsh/sdk-js"].version')
Comment thread
franzwarning marked this conversation as resolved.
Comment on lines +48 to +50

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a fail-closed gate keyed on three unverified assumptions about a repo this one can't see: that play has a package-lock.json (npm, not pnpm/yarn/bun), that it's lockfileVersion 2/3 (v1 has no .packages), and that the sdk is hoisted to the top-level node_modules/@wvdsh/sdk-js key rather than a workspace path like apps/web/node_modules/@wvdsh/sdk-js. If any holds false, jq -er exits non-zero (or the contents call 404s) and every release is permanently blocked — with a bare jq/gh error, not the actionable ::error:: message below, and with a remediation ("bump the pin") that cannot possibly fix it. Note also that CLAUDE.md and scripts/bump-sdk-js.sh both name play/package.json as the source of truth, so the gate reads a different file than the docs. Consider failing with an explicit "could not determine the sdk version play ships" message when the lookup itself fails, so an unreadable play repo is distinguishable from a genuine mismatch.

echo "cli pins $pinned; play $tag ships $shipped"

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The remediation command in this message doesn't run as written — the script isn't executable.

$ git ls-files -s scripts/bump-sdk-js.sh
100644 cd62a35... 0	scripts/bump-sdk-js.sh

Mode 100644, not 100755. So the two ways someone would read "Run scripts/bump-sdk-js.sh 1.3.44" both fail:

  • ./scripts/bump-sdk-js.sh 1.3.44 → bash: ./scripts/bump-sdk-js.sh: Permission denied
  • scripts/bump-sdk-js.sh 1.3.44 → same (and command not found if they drop the path)

Only bash scripts/bump-sdk-js.sh 1.3.44 works. The missing bit is pre-existing (the file landed non-executable in #52, and CLAUDE.md has the same ./scripts/bump-sdk-js.sh wording), but this PR is what makes it load-bearing: this string is the only instruction a maintainer gets at the moment a release is blocked, and it's the first thing they'll paste.

Cheapest fix that makes both this message and CLAUDE.md true is git update-index --chmod=+x scripts/bump-sdk-js.sh in a separate commit. Failing that, spell out the interpreter here:

Suggested change
echo "cli pins $pinned; play $tag ships $shipped"
echo "::error::wavedash dev would inject @wvdsh/sdk-js@$pinned but play $tag ships $shipped. Run bash scripts/bump-sdk-js.sh $shipped, verify a game boots with wavedash dev, then push the bump to retry the release."

(Separately, "then push the bump to retry the release" is still not true — see the trigger-paths thread carried over from the earlier rounds.)

if [ "$pinned" != "$shipped" ]; then
echo "::error::wavedash dev would inject @wvdsh/sdk-js@$pinned but play $tag ships $shipped. Run scripts/bump-sdk-js.sh $shipped, verify a game boots with wavedash dev, then push the bump to retry the release."
Comment thread
franzwarning marked this conversation as resolved.
Comment thread
franzwarning marked this conversation as resolved.
Comment thread
franzwarning marked this conversation as resolved.
Comment thread
franzwarning marked this conversation as resolved.
exit 1
fi
Comment thread
franzwarning marked this conversation as resolved.

Comment thread
franzwarning marked this conversation as resolved.
- name: Create and Push Version Tag
if: steps.tagged.outputs.exists == 'false'
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
git tag -a "${{ steps.get_version.outputs.version }}" -m "Release ${{ steps.get_version.outputs.version }}" || echo "Tag already exists"
git push origin "${{ steps.get_version.outputs.version }}" || echo "Tag already pushed"

git tag -a "${{ steps.get_version.outputs.version }}" -m "Release ${{ steps.get_version.outputs.version }}"
git push origin "${{ steps.get_version.outputs.version }}"
Comment on lines +62 to +63

@claude claude Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dropping the || echo "Tag already exists" / || echo "Tag already pushed" fallbacks makes the check-then-push non-atomic and there's no concurrency: group on the workflow, so a benign duplicate now fails the job red. This is more likely after adding src/dev/sdk-js-version to the path filter, since two runs can now be in flight for the same Cargo.toml version: push A bumps the version to 0.1.93, push B (a pin bump, seconds later) contains that same version, both runs pass ls-remote before either reaches this step — the gate step adds several seconds of API latency in between — A pushes the tag and B dies on "tag already exists". A concurrency: { group: auto-tag, cancel-in-progress: false } block, or re-checking git ls-remote immediately before the push, would close the window.

8 changes: 0 additions & 8 deletions CLAUDE.md
View file
Open in desktop

This file was deleted.

Loading