Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
46 changes: 46 additions & 0 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,49 @@ jobs:
tag_name: v${{ steps.get_version.outputs.VERSION }}
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Rollback release on failure
if: failure()
shell: bash
run: |
VERSION=$(node -e "console.log(JSON.parse(require('fs').readFileSync('./package.json', 'utf8')).version)")
TAG="v$VERSION"
Comment on lines +76 to +77

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.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Version extraction duplicated; consider reusing step output.

The version is already extracted in the get_version step (lines 58-62). You could reference ${{ steps.get_version.outputs.VERSION }} via an environment variable instead of re-parsing package.json. However, since failure() context might not preserve all step outputs reliably, verify this works in your CI environment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish.yml around lines 76 - 77, Duplicate version
extraction: replace the local shell node command that reads package.json (the
VERSION and TAG variables) with the previously computed get_version step output
by using steps.get_version.outputs.VERSION to set VERSION (and then
TAG="v$VERSION"), and ensure the job/step that uses this runs in a context where
step outputs are preserved (confirm behavior when using failure() context in
your CI); update any environment variable references to use that output rather
than re-parsing package.json.

RELEASE_SHA=$(git rev-parse HEAD)
COMMIT_MSG=$(git log -1 --pretty=%B "$RELEASE_SHA")

if echo "$COMMIT_MSG" | grep -q "chore: release v$VERSION"; then
echo "Workflow failed. Attempting to roll back release $TAG..."

# Delete remote tag if it exists
git push origin :refs/tags/"$TAG" || true
Comment on lines +84 to +85

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

GitHub Release artifact is not deleted, only the tag.

The softprops/action-gh-release@v3 action creates a GitHub Release object. Deleting the tag via git push origin :refs/tags/"$TAG" leaves an orphaned release pointing to a non-existent tag. You should also delete the release itself using the GitHub API.

🛠️ Proposed fix to delete the GitHub Release
             # Delete remote tag if it exists
             git push origin :refs/tags/"$TAG" || true
+            
+            # Delete GitHub Release if it exists
+            RELEASE_ID=$(gh release view "$TAG" --json id -q '.id' 2>/dev/null || true)
+            if [ -n "$RELEASE_ID" ]; then
+              gh release delete "$TAG" --yes || echo "Failed to delete GitHub Release"
+            fi

Note: This requires the gh CLI and appropriate GITHUB_TOKEN permissions (already present).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Delete remote tag if it exists
git push origin :refs/tags/"$TAG" || true
# Delete remote tag if it exists
git push origin :refs/tags/"$TAG" || true
# Delete GitHub Release if it exists
RELEASE_ID=$(gh release view "$TAG" --json id -q '.id' 2>/dev/null || true)
if [ -n "$RELEASE_ID" ]; then
gh release delete "$TAG" --yes || echo "Failed to delete GitHub Release"
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish.yml around lines 84 - 85, When deleting the tag
you also need to delete the GitHub Release object created by
softprops/action-gh-release@v3 so it doesn't remain orphaned; after the existing
tag deletion step (git push origin :refs/tags/"$TAG") call the GitHub API/gh CLI
to remove the release matching "$TAG" (e.g., use gh release delete "$TAG" --yes
or the REST API with GITHUB_TOKEN) and ensure the workflow has permission to
delete releases so both the tag and the release are removed.


# Fetch latest remote changes
git fetch origin main
REMOTE_SHA=$(git rev-parse origin/main)

# Ensure local working directory is clean
git reset --hard HEAD

if [ "$REMOTE_SHA" = "$RELEASE_SHA" ]; then
echo "Release commit is the latest on origin/main. Force-pushing HEAD~1..."
if git push origin HEAD~1:main --force; then
echo "Rollback successful: release commit removed."
else
echo "Force-push failed (likely due to branch protection). Creating a revert commit instead..."
git revert --no-edit HEAD
git push origin HEAD:main
Comment on lines +99 to +101

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.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Revert fallback after failed force-push doesn't sync with remote first.

If force-push fails due to branch protection (line 96), the fallback immediately runs git revert --no-edit HEAD on the local state without pulling the latest remote. If branch protection is enabled, there may also be additional commits on origin/main that arrived after git fetch. The subsequent git push origin HEAD:main will fail as a non-fast-forward push.

🐛 Proposed fix to sync before revert
               else
                 echo "Force-push failed (likely due to branch protection). Creating a revert commit instead..."
+                git fetch origin main
+                git checkout main
+                git reset --hard origin/main
                 git revert --no-edit HEAD
-                git push origin HEAD:main
+                git revert --no-edit "$RELEASE_SHA"
+                git push origin main
               fi

Wait—after reset to origin/main, the release commit may no longer be HEAD. The revert target should be $RELEASE_SHA explicitly, and you need to verify it's still an ancestor.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish.yml around lines 99 - 101, When the force-push
fallback runs, first sync local with remote and explicitly revert the original
release commit instead of assuming HEAD: run git fetch origin, then verify that
$RELEASE_SHA is still an ancestor of origin/main (git merge-base --is-ancestor
$RELEASE_SHA origin/main); if the check passes run git checkout -B
revert-fallback origin/main && git revert --no-edit $RELEASE_SHA && git push
origin HEAD:main, otherwise fail the job with a clear error message so you don't
attempt a non-fast-forward push; reference the variables/refs RELEASE_SHA,
origin/main and the git revert command in the workflow.

fi
Comment on lines +94 to +102

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

TOCTOU race between fetch and force-push.

Between git fetch origin main (line 88) and the force-push (line 96), another commit could land on origin/main. The $REMOTE_SHA == $RELEASE_SHA check would pass based on stale data, but the force-push would then overwrite unrelated commits. Consider re-fetching immediately before the push or using --force-with-lease to ensure the expected ref hasn't moved.

🔒 Proposed fix using --force-with-lease
-              if git push origin HEAD~1:main --force; then
+              if git push origin HEAD~1:main --force-with-lease=main:"$REMOTE_SHA"; then

This ensures the push only succeeds if origin/main still points to $REMOTE_SHA.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish.yml around lines 94 - 102, The push logic has a
TOCTOU race: after `git fetch origin main` and checking `REMOTE_SHA` ==
`RELEASE_SHA`, another commit could land before `git push origin HEAD~1:main
--force`; update the branch protection by either re-fetching the remote ref
immediately before pushing or (preferably) replace the unconditional `git push
origin HEAD~1:main --force` with a safe push using `--force-with-lease` so the
push will only succeed if `origin/main` still equals `REMOTE_SHA`; ensure
references to `REMOTE_SHA`, `RELEASE_SHA`, the `git fetch origin main` step, and
the `git push origin HEAD~1:main --force` command are adjusted accordingly.

else
if git merge-base --is-ancestor "$RELEASE_SHA" origin/main; then
echo "Release commit exists on origin/main but is not the latest. Reverting it..."
git checkout main
git pull origin main
git revert --no-edit "$RELEASE_SHA"
git push origin HEAD:main
else
echo "Release commit was not pushed to origin/main. No rollback needed."
fi
fi
else
echo "No release commit found on HEAD. No rollback needed."
fi
Comment on lines +72 to +116

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.

🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

No handling for workflow cancellation mid-rollback.

If the workflow is cancelled while the rollback script is executing, the repository could be left in a partially rolled-back state (e.g., tag deleted but branch not reverted). Consider adding a trap to handle SIGTERM/SIGINT gracefully, or at minimum, document this risk.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish.yml around lines 72 - 116, The rollback script
(job step "Rollback release on failure") lacks signal handling and can leave the
repo in a partial state if canceled mid-run; add a shell trap for SIGINT and
SIGTERM at the start of the step that records current state (e.g., TAG,
RELEASE_SHA, REMOTE_SHA), sets a flag, and runs a cleanup handler which attempts
to revert any partial actions (for example, re-push a deleted tag or
abort/restore working tree with git reset --hard and re-checkout origin/main)
before exiting; ensure the trap wraps the main logic around the variables TAG
and RELEASE_SHA and that all git operations check for the cleanup-flag so the
handler can safely restore or bail out cleanly.

2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"skipLibCheck": true,
"forceConsistentCasingInFileNames": true,
"jsx": "react",
"ignoreDeprecations": "5.0"
"ignoreDeprecations": "6.0"
},
"include": ["src/**/*"],
"exclude": ["node_modules", "dist", "**/*.test.ts"]
Expand Down