Add automated rollback logic to publish workflow and update TypeScript settings#109
Conversation
…cript ignoreDeprecations to 6.0
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Rollback release on failure step .github/workflows/publish.yml |
New failure-only workflow step that reverses release effects by deleting the remote tag and conditionally restoring the main branch via force-push or revert commits, with logic to detect release commit presence and HEAD position. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Rationale: Shell command logic is high-density with multiple git operations, conditional branches, and string substitution from workflow context. Security considerations around command injection, git authorization, and branch history mutations require careful inspection. The step's role in disaster recovery adds criticality despite its single-file footprint.
Possibly related PRs
- KDM-cli/kdm-cli#45: Directly related workflow modification; both PRs alter the publish/release pipeline in
.github/workflows/publish.yml.
Suggested labels
ci/cd
Poem
🚨 When releases fail and chaos blooms,
This step sweeps clean the git-stained rooms—
Tags deleted, branches mended fast,
Main restored to what it was last. ✨
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | The title mentions 'Add automated rollback logic to publish workflow' which directly corresponds to the main change (conditional rollback step in publish.yml), and also references 'update TypeScript settings' which is not present in the changeset. | Remove the 'update TypeScript settings' portion from the title, as the changeset only modifies the publish workflow with rollback logic. Consider: 'Add automated rollback logic to publish workflow'. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 @coderabbitai help to get the list of available commands and usage tips.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Our agent can fix these. Install it.
No application code in the PR — skipped Code Health checks.
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/publish.yml (1)
53-56:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftnpm publish is irreversible; rollback leaves published package on registry.
The rollback step triggers
if: failure(), but oncenpm publishsucceeds (line 54), subsequent step failures (e.g., GitHub Release creation) will initiate rollback. However, npm packages cannot be unpublished after 72 hours, and even within that window, unpublishing is discouraged. This creates an inconsistent state: the tag and commit are rolled back, but the package version exists on npm.Consider restructuring the workflow to minimize this window—e.g., create the GitHub Release before publishing to npm, or add an explicit
npm unpublishattempt in the rollback (with appropriate caveats about npm's deprecation policy).Also applies to: 72-73
🤖 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 53 - 56, The workflow currently runs the "Publish to npm" step (npm publish --access public) before creating the GitHub Release and relies on a rollback triggered by if: failure(), which can leave the package published even if other steps fail; change the job order so the GitHub Release (and any other post-release validations) are created/verified before the "Publish to npm" step (make "Publish to npm" the last step), or alternatively add an explicit rollback action that attempts npm unpublish (with a clear warning/comment about npm's unpublish policy) in the failure handler—refer to the "Publish to npm" step name and the failure() conditional to locate where to reorder or add the npm-unpublish rollback.
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 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.
- Around line 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.
- Around line 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.
- Around line 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.
- Around line 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.
---
Outside diff comments:
In @.github/workflows/publish.yml:
- Around line 53-56: The workflow currently runs the "Publish to npm" step (npm
publish --access public) before creating the GitHub Release and relies on a
rollback triggered by if: failure(), which can leave the package published even
if other steps fail; change the job order so the GitHub Release (and any other
post-release validations) are created/verified before the "Publish to npm" step
(make "Publish to npm" the last step), or alternatively add an explicit rollback
action that attempts npm unpublish (with a clear warning/comment about npm's
unpublish policy) in the failure handler—refer to the "Publish to npm" step name
and the failure() conditional to locate where to reorder or add the
npm-unpublish rollback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 26747428-80dc-41c3-9df8-370ce5a10f35
⛔ Files ignored due to path filters (1)
tsconfig.jsonis excluded by none and included by none
📒 Files selected for processing (1)
.github/workflows/publish.yml
| - 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" | ||
| 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 | ||
|
|
||
| # 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 | ||
| fi | ||
| 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 |
There was a problem hiding this comment.
🧹 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.
| VERSION=$(node -e "console.log(JSON.parse(require('fs').readFileSync('./package.json', 'utf8')).version)") | ||
| TAG="v$VERSION" |
There was a problem hiding this comment.
🧹 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.
| # Delete remote tag if it exists | ||
| git push origin :refs/tags/"$TAG" || true |
There was a problem hiding this comment.
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"
+ fiNote: 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.
| # 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.
| 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 | ||
| fi |
There was a problem hiding this comment.
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"; thenThis 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.
| echo "Force-push failed (likely due to branch protection). Creating a revert commit instead..." | ||
| git revert --no-edit HEAD | ||
| git push origin HEAD:main |
There was a problem hiding this comment.
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
fiWait—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.
1 participant
Summary by CodeRabbit