From d20df88470cdb4359dbee400958885ba7470da0f Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 30 Mar 2026 19:38:52 -0700 Subject: [PATCH] fix: add idempotency guards to /ship Steps 4, 7, 8 (#649) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If git push succeeds but gh pr create fails, re-running /ship would double-bump VERSION and duplicate CHANGELOG entries. Now: - Step 4: check if VERSION already differs from base branch - Step 7: fetch only the specific branch, skip push if already up to date - Step 8: if PR exists, update body via gh pr edit instead of creating duplicate No CHANGELOG guard needed — Step 5 is already idempotent by design ("replace existing entries with one unified entry"). Co-Authored-By: Claude Opus 4.6 (1M context) --- ship/SKILL.md | 39 +++++++++++++++++++++++++++++++++++++-- ship/SKILL.md.tmpl | 39 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/ship/SKILL.md b/ship/SKILL.md index 4519b6e2..a4140ff2 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -1755,6 +1755,17 @@ already knows. A good test: would this insight save time in a future session? If ## Step 4: Version bump (auto-decide) +**Idempotency check:** Before bumping, compare VERSION against the base branch. + +```bash +BASE_VERSION=$(git show origin/:VERSION 2>/dev/null || echo "0.0.0.0") +CURRENT_VERSION=$(cat VERSION 2>/dev/null || echo "0.0.0.0") +echo "BASE: $BASE_VERSION HEAD: $CURRENT_VERSION" +if [ "$CURRENT_VERSION" != "$BASE_VERSION" ]; then echo "ALREADY_BUMPED"; fi +``` + +If output shows `ALREADY_BUMPED`, VERSION was already bumped on this branch (prior `/ship` run). Skip the rest of Step 4 and use the current VERSION. Otherwise proceed with the bump. + 1. Read the current `VERSION` file (4-digit format: `MAJOR.MINOR.PATCH.MICRO`) 2. **Auto-decide the bump level based on the diff:** @@ -1934,7 +1945,17 @@ Claiming work is complete without verification is dishonesty, not efficiency. ## Step 7: Push -Push to the remote with upstream tracking: +**Idempotency check:** Check if the branch is already pushed and up to date. + +```bash +git fetch origin 2>/dev/null +LOCAL=$(git rev-parse HEAD) +REMOTE=$(git rev-parse origin/ 2>/dev/null || echo "none") +echo "LOCAL: $LOCAL REMOTE: $REMOTE" +[ "$LOCAL" = "$REMOTE" ] && echo "ALREADY_PUSHED" || echo "PUSH_NEEDED" +``` + +If `ALREADY_PUSHED`, skip the push. Otherwise push with upstream tracking: ```bash git push -u origin @@ -1944,7 +1965,21 @@ git push -u origin ## Step 8: Create PR/MR -Create a pull request (GitHub) or merge request (GitLab) using the platform detected in Step 0. +**Idempotency check:** Check if a PR/MR already exists for this branch. + +**If GitHub:** +```bash +gh pr view --json url,number -q '"PR #\(.number): \(.url)"' 2>/dev/null || echo "NO_PR" +``` + +**If GitLab:** +```bash +glab mr view -F json 2>/dev/null && echo "MR_EXISTS" || echo "NO_MR" +``` + +If a PR/MR already exists: **update** the PR body with the latest test results, coverage, and review findings using `gh pr edit --body "..."` (GitHub) or `glab mr update -d "..."` (GitLab). Print the existing URL and continue to Step 8.5. + +If no PR/MR exists: create a pull request (GitHub) or merge request (GitLab) using the platform detected in Step 0. The PR/MR body should contain these sections: diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index 993a67a5..52bce243 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -328,6 +328,17 @@ For each classified comment: ## Step 4: Version bump (auto-decide) +**Idempotency check:** Before bumping, compare VERSION against the base branch. + +```bash +BASE_VERSION=$(git show origin/:VERSION 2>/dev/null || echo "0.0.0.0") +CURRENT_VERSION=$(cat VERSION 2>/dev/null || echo "0.0.0.0") +echo "BASE: $BASE_VERSION HEAD: $CURRENT_VERSION" +if [ "$CURRENT_VERSION" != "$BASE_VERSION" ]; then echo "ALREADY_BUMPED"; fi +``` + +If output shows `ALREADY_BUMPED`, VERSION was already bumped on this branch (prior `/ship` run). Skip the rest of Step 4 and use the current VERSION. Otherwise proceed with the bump. + 1. Read the current `VERSION` file (4-digit format: `MAJOR.MINOR.PATCH.MICRO`) 2. **Auto-decide the bump level based on the diff:** @@ -467,7 +478,17 @@ Claiming work is complete without verification is dishonesty, not efficiency. ## Step 7: Push -Push to the remote with upstream tracking: +**Idempotency check:** Check if the branch is already pushed and up to date. + +```bash +git fetch origin 2>/dev/null +LOCAL=$(git rev-parse HEAD) +REMOTE=$(git rev-parse origin/ 2>/dev/null || echo "none") +echo "LOCAL: $LOCAL REMOTE: $REMOTE" +[ "$LOCAL" = "$REMOTE" ] && echo "ALREADY_PUSHED" || echo "PUSH_NEEDED" +``` + +If `ALREADY_PUSHED`, skip the push. Otherwise push with upstream tracking: ```bash git push -u origin @@ -477,7 +498,21 @@ git push -u origin ## Step 8: Create PR/MR -Create a pull request (GitHub) or merge request (GitLab) using the platform detected in Step 0. +**Idempotency check:** Check if a PR/MR already exists for this branch. + +**If GitHub:** +```bash +gh pr view --json url,number -q '"PR #\(.number): \(.url)"' 2>/dev/null || echo "NO_PR" +``` + +**If GitLab:** +```bash +glab mr view -F json 2>/dev/null && echo "MR_EXISTS" || echo "NO_MR" +``` + +If a PR/MR already exists: **update** the PR body with the latest test results, coverage, and review findings using `gh pr edit --body "..."` (GitHub) or `glab mr update -d "..."` (GitLab). Print the existing URL and continue to Step 8.5. + +If no PR/MR exists: create a pull request (GitHub) or merge request (GitLab) using the platform detected in Step 0. The PR/MR body should contain these sections: