From 70b962a4c873308e5760a548afec090ea94fea3b Mon Sep 17 00:00:00 2001
From: Cheney <970320820@qq.com>
Date: Tue, 12 May 2026 17:15:35 +0800
Subject: [PATCH] fix(cli): harden release pipeline per PR #422 review

1. npm version check: three-state logic (exists/missing/error) to prevent
   silent skip on network failures, registry 5xx, or auth issues.

2. workflow_dispatch: checkout the specified tag and validate SHA matches,
   preventing builds from wrong ref.

3. Atomic push: use `git push --atomic` and detect unpushed tags via
   `git ls-remote` instead of `--no-merged` (catches branch-pushed-but-
   tag-failed state).
---
 .github/workflows/release-cli.yml | 55 ++++++++++++++++++++++++--
 scripts/publish-cli.sh            | 17 ++++++--
 scripts/tests/publish-cli-test.sh | 66 +++++++++++++++++++++++++++++--
 3 files changed, 129 insertions(+), 9 deletions(-)

diff --git a/.github/workflows/release-cli.yml b/.github/workflows/release-cli.yml
index 1db9924a..16495638 100644
--- a/.github/workflows/release-cli.yml
+++ b/.github/workflows/release-cli.yml
@@ -29,6 +29,34 @@ jobs:
     steps:
       - name: Check out repository
         uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.inputs.tag || github.ref }}
+
+      - name: Validate tag (workflow_dispatch only)
+        if: github.event_name == 'workflow_dispatch'
+        run: |
+          TAG="${{ github.event.inputs.tag }}"
+
+          # Verify tag exists
+          if ! git rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then
+            echo "ERROR: Tag '$TAG' does not exist in the repository" >&2
+            exit 1
+          fi
+
+          # Verify current checkout matches the tag
+          TAG_SHA=$(git rev-parse "refs/tags/$TAG^{commit}")
+          CURRENT_SHA=$(git rev-parse HEAD)
+
+          if [ "$TAG_SHA" != "$CURRENT_SHA" ]; then
+            echo "ERROR: Current checkout SHA does not match tag '$TAG'" >&2
+            echo "  Tag SHA:     $TAG_SHA" >&2
+            echo "  Current SHA: $CURRENT_SHA" >&2
+            echo "" >&2
+            echo "This indicates the checkout did not switch to the specified tag." >&2
+            exit 1
+          fi
+
+          echo "✓ Tag '$TAG' validated (SHA: $TAG_SHA)"
 
       - name: Set up Bun
         uses: oven-sh/setup-bun@v2
@@ -140,12 +168,33 @@ jobs:
           PACKAGE_NAME="${{ needs.build-and-test.outputs.package_name }}"
           VERSION="${{ needs.build-and-test.outputs.version }}"
 
-          if npm view "${PACKAGE_NAME}@${VERSION}" version --registry "$NPM_REGISTRY" 2>&1 | grep -Eiq '(E404|404 Not Found|is not in this registry|Not found)'; then
+          # Three-state check: success (exists) / 404 (missing) / error (fail job)
+          set +e
+          NPM_OUTPUT=$(npm view "${PACKAGE_NAME}@${VERSION}" version --registry "$NPM_REGISTRY" 2>&1)
+          NPM_EXIT_CODE=$?
+          set -e
+
+          if [ $NPM_EXIT_CODE -eq 0 ]; then
+            # Success: version exists on registry
+            echo "exists=true" >> "$GITHUB_OUTPUT"
+            echo "Version $VERSION already exists on registry, skipping publish"
+          elif echo "$NPM_OUTPUT" | grep -Eiq '(E404|404 Not Found|is not in this registry|Not found)'; then
+            # Explicit 404: version does not exist
             echo "exists=false" >> "$GITHUB_OUTPUT"
             echo "Version $VERSION does not exist on registry, proceeding with publish"
           else
-            echo "exists=true" >> "$GITHUB_OUTPUT"
-            echo "Version $VERSION already exists on registry, skipping publish"
+            # Uncertain state: network error, auth failure, registry error, etc.
+            echo "ERROR: Failed to check npm registry (exit code: $NPM_EXIT_CODE)" >&2
+            echo "Output: $NPM_OUTPUT" >&2
+            echo "" >&2
+            echo "This could be due to:" >&2
+            echo "  - Network connectivity issues" >&2
+            echo "  - Registry service errors (5xx)" >&2
+            echo "  - Authentication/authorization failures" >&2
+            echo "  - DNS or TLS problems" >&2
+            echo "" >&2
+            echo "Cannot safely determine if version exists. Failing job to prevent silent skip." >&2
+            exit 1
           fi
 
       - name: Configure npm authentication
diff --git a/scripts/publish-cli.sh b/scripts/publish-cli.sh
index 0e9c31b9..262414dd 100755
--- a/scripts/publish-cli.sh
+++ b/scripts/publish-cli.sh
@@ -56,7 +56,18 @@ git -C "$REPO_ROOT" fetch --tags --prune origin
 
 log_stage "checking for unpushed release artifacts"
 UNPUSHED_COMMITS="$(git -C "$REPO_ROOT" log --oneline origin/"$CURRENT_BRANCH"..HEAD 2>/dev/null || true)"
-UNPUSHED_RELEASE_TAGS="$(git -C "$REPO_ROOT" tag --list 'cli-v*' --no-merged origin/"$CURRENT_BRANCH" 2>/dev/null || true)"
+
+# Detect local cli-v* tags that don't exist on origin.
+# This catches both: (a) commit not pushed + tag not pushed, and
+# (b) commit pushed but tag push failed (where --no-merged would miss it).
+UNPUSHED_RELEASE_TAGS=""
+while IFS= read -r local_tag; do
+  [[ -z "$local_tag" ]] && continue
+  if ! git -C "$REPO_ROOT" ls-remote --exit-code --tags origin "refs/tags/$local_tag" >/dev/null 2>&1; then
+    UNPUSHED_RELEASE_TAGS="${UNPUSHED_RELEASE_TAGS:+$UNPUSHED_RELEASE_TAGS
+}$local_tag"
+  fi
+done < <(git -C "$REPO_ROOT" tag --list 'cli-v*' 2>/dev/null)
 
 if [[ -n "$UNPUSHED_COMMITS" ]] || [[ -n "$UNPUSHED_RELEASE_TAGS" ]]; then
   echo "" >&2
@@ -152,8 +163,8 @@ git -C "$REPO_ROOT" commit -m "chore(cli): bump version to $NEW_VERSION"
 log_stage "creating tag $TAG"
 git -C "$REPO_ROOT" tag "$TAG"
 
-log_stage "pushing commit and tag to origin"
-git -C "$REPO_ROOT" push origin "$CURRENT_BRANCH" "$TAG"
+log_stage "pushing commit and tag to origin (atomic)"
+git -C "$REPO_ROOT" push --atomic origin "$CURRENT_BRANCH" "$TAG"
 
 log_stage "release triggered — CI workflow will build and publish"
 log_stage "watch progress at: https://github.com/iflytek/skillhub/actions/workflows/release-cli.yml"
diff --git a/scripts/tests/publish-cli-test.sh b/scripts/tests/publish-cli-test.sh
index d4cbe7d3..1729dd9c 100755
--- a/scripts/tests/publish-cli-test.sh
+++ b/scripts/tests/publish-cli-test.sh
@@ -120,11 +120,13 @@ run_publish() {
   local input="${3-}"
   local status=0
   if [[ -n "$input" ]]; then
-    printf '%s' "$input" | REPO_ROOT="$repo" PATH="$repo/bin:$PATH" \
+    printf '%s' "$input" | env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE \
+      REPO_ROOT="$repo" PATH="$repo/bin:$PATH" \
       bash "$repo/scripts/publish-cli.sh" "$bump" \
       >"$repo/stdout.log" 2>"$repo/stderr.log" || status=$?
   else
-    REPO_ROOT="$repo" PATH="$repo/bin:$PATH" \
+    env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE \
+      REPO_ROOT="$repo" PATH="$repo/bin:$PATH" \
       bash "$repo/scripts/publish-cli.sh" "$bump" \
       >"$repo/stdout.log" 2>"$repo/stderr.log" || status=$?
   fi
@@ -272,7 +274,7 @@ grep -F "release triggered" "$REPO8/stdout.log" >/dev/null
 # ----------------------------------------------------------------------------
 # Test 9: push failure → script exits non-zero (commit + tag stay local)
 #
-# Break origin to force `git push origin main cli-vX.Y.Z` to fail.
+# Break origin to force `git push --atomic origin main cli-vX.Y.Z` to fail.
 # ----------------------------------------------------------------------------
 echo "[test] push failure surfaces error"
 REPO9="$(new_tmp)"
@@ -286,4 +288,62 @@ git -C "$REPO9" rev-parse "cli-v0.6.1" >/dev/null \
 git -C "$REPO9" log --oneline | grep -F "chore(cli): bump version to 0.6.1" >/dev/null \
   || fail "local bump commit missing after push failure"
 
+# ----------------------------------------------------------------------------
+# Test 10: unpushed detection catches "branch pushed, tag not pushed" state
+#
+# Simulate: commit is on origin/main, local tag exists but was never pushed.
+# The old `--no-merged` approach would miss this because the tagged commit is
+# already reachable from origin/main. The new ls-remote approach catches it.
+# ----------------------------------------------------------------------------
+echo "[test] unpushed detection catches tag-only failure"
+REPO10="$(new_tmp)"
+init_repo "$REPO10" "0.7.0"
+# Manually create a release commit and push only the branch (not the tag)
+cd "$REPO10/cli"
+node -e "
+  const fs = require('fs');
+  const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
+  pkg.version = '0.7.1';
+  fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2) + '\n');
+"
+cd "$REPO10"
+git -C "$REPO10" add cli/package.json
+git -C "$REPO10" commit -q -m "chore(cli): bump version to 0.7.1"
+git -C "$REPO10" tag "cli-v0.7.1"
+git -C "$REPO10" push -q origin main
+# Tag NOT pushed — simulates atomic push partial failure recovery
+# (or a scenario where user manually pushed branch but tag failed)
+status="$(run_publish "$REPO10" "patch")"
+[[ "$status" -ne 0 ]] || fail "expected non-zero exit when unpushed tag detected"
+grep -F "Unpushed tags" "$REPO10/stderr.log" >/dev/null \
+  || { cat "$REPO10/stderr.log" >&2; fail "expected unpushed tag warning"; }
+grep -F "cli-v0.7.1" "$REPO10/stderr.log" >/dev/null \
+  || fail "expected cli-v0.7.1 in unpushed tag warning"
+
+# ----------------------------------------------------------------------------
+# Test 11: --atomic flag is actually passed to git push
+#
+# Use a git wrapper to capture the push command and verify --atomic is present.
+# ----------------------------------------------------------------------------
+echo "[test] push uses --atomic flag"
+REPO11="$(new_tmp)"
+init_repo "$REPO11" "0.8.0"
+# Create a git wrapper that logs push commands
+mkdir -p "$REPO11/bin-git"
+cat >"$REPO11/bin-git/git" <<'WRAPPER'
+#!/usr/bin/env bash
+if [[ "${1:-}" == "push" ]]; then
+  echo "GIT_PUSH_ARGS: $*" >> "$REPO_ROOT/git-push-log.txt"
+fi
+exec /usr/bin/git "$@"
+WRAPPER
+chmod +x "$REPO11/bin-git/git"
+status="$(env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE \
+  REPO_ROOT="$REPO11" PATH="$REPO11/bin-git:$REPO11/bin:$PATH" \
+  printf 'y\n' | bash "$REPO11/scripts/publish-cli.sh" "patch" \
+  >"$REPO11/stdout.log" 2>"$REPO11/stderr.log" && echo 0 || echo $?)"
+[[ "$status" -eq 0 ]] || { cat "$REPO11/stderr.log" >&2; fail "expected success, got $status"; }
+grep -F -- "--atomic" "$REPO11/git-push-log.txt" >/dev/null \
+  || { cat "$REPO11/git-push-log.txt" >&2; fail "git push did not include --atomic flag"; }
+
 echo "all tests passed"