fix(cli): address PR #441 review findings

- Fix ERR trap bypass: remove `if !` wrapper around `gh pr create` so set -e triggers the trap and prints pushed-stage recovery instructions - Fix command injection: all node -e/-p calls now use process.env instead of interpolating shell variables into JS string literals - Rewrite cli/RELEASE.md to document the new PR-based release flow - Rewrite scripts/tests/publish-cli-test.sh with 10 tests covering the new flow (stubs for bun/gh, pre-flight checks, happy path, cleanup state machine stages)
2026-08-28 11:25:00 +00:00 · 2026-05-15 17:06:10 +08:00 · 2026-05-15 17:06:10 +08:00 · efcf0ee19d
commit efcf0ee19d
parent 4c7285b3f5
3 changed files with 295 additions and 174 deletions
--- a/cli/RELEASE.md
+++ b/cli/RELEASE.md
@ -2,7 +2,14 @@

 ## Overview

-CLI releases are fully automated. Running `make publish-cli` on a clean `main` branch bumps the version, commits, creates a `cli-vX.Y.Z` tag, and pushes everything to origin. The GitHub Actions workflow [`release-cli.yml`](../.github/workflows/release-cli.yml) listens for the tag and handles build, test, npm publish, and GitHub Release creation.
+CLI releases use a PR-based flow. Running `make publish-cli` on a clean `main` branch:
+
+1. Runs local build-and-test (lint, typecheck, test, build)
+2. Computes the next version from the latest `cli-v*` tag
+3. Creates a `release/cli-vX.Y.Z` branch with the version bump committed
+4. Pushes the branch and opens a PR to `main`
+
+After the PR is merged, you manually tag and push — the tag triggers [`release-cli.yml`](../.github/workflows/release-cli.yml) which builds, publishes to npm, and creates a GitHub Release.

 ## Prerequisites

@ -21,8 +28,9 @@ Configure in GitHub repository → Settings → Secrets and variables → Action

 ### Local Environment

- `node` and `npm` installed (the script uses `npm version` to bump)
- `git` installed with push access to the repository
+- `node` and `bun` installed
+- `gh` CLI installed and authenticated (`gh auth login`)
+- `git` with push access to the repository
 - On the `main` branch with a clean working tree

 ### Package Configuration
@ -40,7 +48,7 @@ In [`cli/package.json`](./package.json):

 ## Release Process

-### One-shot Release
+### Step 1: Run the publish script

 From the repository root, on a clean `main` branch:

@ -52,15 +60,29 @@ make publish-cli-major   # major: 0.1.5 -> 1.0.0

 [`scripts/publish-cli.sh`](../scripts/publish-cli.sh) performs the following steps:

-1. Verify the working tree is clean
-2. Require the current branch to be `main`, otherwise abort
+1. Verify `gh` CLI is installed and authenticated
+2. Verify the working tree is clean and on `main`
 3. `git pull --ff-only` from `origin/main`
-4. Fetch remote tags and align `package.json` with the latest `cli-v*` tag
-5. Compute the new version via `npm version <bump>`
-6. Verify the new tag does not exist locally or on origin
-7. After interactive confirmation: commit the bump, create the `cli-vX.Y.Z` tag, push both commit and tag to origin
+4. Run full local build-and-test (lint, typecheck, test, build)
+5. Compute the next version from the latest `cli-v*` tag
+6. Verify the tag and release branch don't already exist
+7. After interactive confirmation: create release branch, commit version bump, push, and open PR

-Pushing the tag triggers CI — no further manual action required.
+### Step 2: Merge the PR
+
+Review and merge the PR on GitHub as usual.
+
+### Step 3: Tag and push
+
+After the PR is merged:
+
+```bash
+git pull origin main
+git tag cli-vX.Y.Z       # replace with the actual version
+git push origin cli-vX.Y.Z
+```
+
+Pushing the tag triggers CI which builds, publishes to npm, and creates a GitHub Release.

 ### CI Workflow

@ -104,6 +126,22 @@ From the Actions UI:
 2. Enter an existing tag name matching `cli-vX.Y.Z`
 3. Optionally enable skip npm publish

+## Error Recovery
+
+The script uses a cleanup state machine. If it fails at different stages:
+
+- **Before push**: release branch is deleted locally, you're returned to `main`
+- **After push, before PR**: the script prints recovery instructions (open PR manually or delete the remote branch)
+- **After PR opened**: success — no cleanup needed
+
+If you need to manually clean up a failed release:
+
+```bash
+git checkout main
+git branch -D release/cli-vX.Y.Z           # delete local branch
+git push origin --delete release/cli-vX.Y.Z # delete remote branch (if pushed)
+```
+
 ## Troubleshooting

 ### `releases must be cut from 'main'`
@ -118,6 +156,10 @@ Commit or stash local changes first.

 The previous release didn't clean up, or someone else released the same version. Check `git tag --list 'cli-v*'` and remote tags, then retry with a higher version.

+### `branch release/cli-vX.Y.Z already exists`
+
+A previous release attempt left a stale branch. Delete it locally and/or on origin, then retry.
+
 ### npm Publish Fails

 - **403 with 2FA message**: `NPM_TOKEN` is not an Automation Token, or bypass 2FA is not enabled — regenerate with the correct type
--- a/scripts/publish-cli.sh
+++ b/scripts/publish-cli.sh
@ -142,17 +142,17 @@ if [[ -n "$LATEST_TAG" ]]; then
  BASE_VERSION="${LATEST_TAG#cli-v}"
  log_stage "baseline: $BASE_VERSION (from $LATEST_TAG)"
 else
-  BASE_VERSION="$(node -p "require('$PACKAGE_JSON').version")"
+  BASE_VERSION="$(PACKAGE_JSON="$PACKAGE_JSON" node -p "require(process.env.PACKAGE_JSON).version")"
  log_stage "no cli-v* tags found, baseline: $BASE_VERSION (from package.json)"
 fi

-NEW_VERSION="$(node -e "
-  const v = '$BASE_VERSION'.split('.').map(Number);
+NEW_VERSION="$(BASE_VERSION="$BASE_VERSION" BUMP_TYPE="$BUMP_TYPE" node -e "
+  const v = process.env.BASE_VERSION.split('.').map(Number);
  if (v.length !== 3 || v.some(Number.isNaN)) {
-    console.error('invalid baseline version: $BASE_VERSION');
+    console.error('invalid baseline version: ' + process.env.BASE_VERSION);
    process.exit(1);
  }
-  const t = '$BUMP_TYPE';
+  const t = process.env.BUMP_TYPE;
  if (t === 'patch') v[2]++;
  else if (t === 'minor') { v[1]++; v[2] = 0; }
  else if (t === 'major') { v[0]++; v[1] = 0; v[2] = 0; }
@ -225,11 +225,11 @@ git -C "$REPO_ROOT" checkout -b "$RELEASE_BRANCH"
 CLEANUP_STAGE="on-release"

 log_stage "writing version $NEW_VERSION to package.json"
-node -e "
+NEW_VERSION="$NEW_VERSION" PACKAGE_JSON="$PACKAGE_JSON" node -e "
  const fs = require('fs');
-  const pkg = JSON.parse(fs.readFileSync('$PACKAGE_JSON', 'utf8'));
-  pkg.version = '$NEW_VERSION';
-  fs.writeFileSync('$PACKAGE_JSON', JSON.stringify(pkg, null, 2) + '\n');
+  const pkg = JSON.parse(fs.readFileSync(process.env.PACKAGE_JSON, 'utf8'));
+  pkg.version = process.env.NEW_VERSION;
+  fs.writeFileSync(process.env.PACKAGE_JSON, JSON.stringify(pkg, null, 2) + '\n');
 "

 log_stage "regenerating pkg-info.ts with new version"
--- a/scripts/tests/publish-cli-test.sh
+++ b/scripts/tests/publish-cli-test.sh
@ -2,10 +2,11 @@

 # Integration tests for scripts/publish-cli.sh.
 #
-# The script bumps cli/package.json, commits, tags `cli-vX.Y.Z`, and pushes
-# both refs to origin. These tests build a self-contained fake repo for each
-# scenario, using a real bare repository as origin so git fetch / pull / push
-# are actually exercised. `npm` is stubbed to keep `npm version` deterministic.
+# The script runs local build-and-test (lint, typecheck, test, build), bumps
+# cli/package.json, pushes a `release/cli-vX.Y.Z` branch, and opens a PR via
+# `gh pr create`. These tests build a self-contained fake repo per scenario,
+# using a real bare repository as origin so git fetch/pull/push are actually
+# exercised. `bun` and `gh` are stubbed to keep the heavy steps deterministic.

 set -euo pipefail

@ -37,8 +38,10 @@ fail() {
 #
 # Builds a minimal repo with:
 #   - cli/package.json at the requested version
+#   - cli/src/generated/pkg-info.ts (committed, so script's checkout works)
 #   - scripts/publish-cli.sh (the script under test)
-#   - bin/npm stub that implements `npm version <bump> --no-git-tag-version`
+#   - bin/bun stub: handles install / run lint|typecheck|build|<codegen> / test
+#   - bin/gh stub: handles `auth status` and `pr create` (logs to gh-stub.log)
 #   - a sibling bare repo as `origin`
 #   - HEAD on `main` with the init commit already pushed
 #   - optional pre-seeded `cli-v*` tags (created locally AND on origin)
@ -53,7 +56,7 @@ init_repo() {
  local origin="$repo.origin.git"
  TMP_DIRS+=("$origin")

-  mkdir -p "$repo/cli" "$repo/scripts" "$repo/bin"
+  mkdir -p "$repo/cli/src/generated" "$repo/scripts" "$repo/bin"
  cp "$PUBLISH_SCRIPT" "$repo/scripts/publish-cli.sh"

  cat >"$repo/cli/package.json" <<EOF
@ -66,40 +69,103 @@ init_repo() {
 }
 EOF

-  # Stub `npm`: only `npm version <patch|minor|major> --no-git-tag-version` is
-  # supported. Mutates package.json in cwd and echoes `vX.Y.Z` (matching real
-  # npm behaviour the script depends on).
-  cat >"$repo/bin/npm" <<'EOF'
+  cat >"$repo/cli/src/generated/pkg-info.ts" <<EOF
+// Generated by scripts/generate-pkg-info.ts - do not edit by hand.
+export const PKG_NAME = "@astron-team/skillhub"
+export const PKG_VERSION = "$version"
+EOF
+
+  # Stub `bun`: handles every subcommand the publish script invokes.
+  # Failure injection via BUN_FAIL_AT (substring match against full args).
+  cat >"$repo/bin/bun" <<'BUN_EOF'
 #!/usr/bin/env bash
 set -euo pipefail
-if [[ "${1:-}" != "version" ]]; then
-  echo "npm stub: unsupported subcommand: $*" >&2
+
+if [[ -n "${BUN_FAIL_AT:-}" ]] && [[ "$*" == *"$BUN_FAIL_AT"* ]]; then
+  echo "stub bun: forced failure at: $*" >&2
  exit 1
 fi
-BUMP="$2"
-node - "$PWD/package.json" "$BUMP" <<'NODE'
-const fs = require("fs");
-const path = process.argv[2];
-const bump = process.argv[3];
-const pkg = JSON.parse(fs.readFileSync(path, "utf8"));
-const parts = pkg.version.split(".").map(Number);
-if (bump === "patch") parts[2] += 1;
-else if (bump === "minor") { parts[1] += 1; parts[2] = 0; }
-else if (bump === "major") { parts[0] += 1; parts[1] = 0; parts[2] = 0; }
-else throw new Error("unexpected bump: " + bump);
-const next = parts.join(".");
-pkg.version = next;
-fs.writeFileSync(path, JSON.stringify(pkg, null, 2) + "\n");
-console.log("v" + next);
-NODE
-EOF
-  chmod +x "$repo/bin/npm"

-  # Ignore test scaffolding files so they don't make `git status` dirty.
+case "${1:-}" in
+  install)
+    exit 0
+    ;;
+  test)
+    exit 0
+    ;;
+  run)
+    case "${2:-}" in
+      lint|typecheck|build)
+        exit 0
+        ;;
+      scripts/generate-pkg-info.ts)
+        node -e "
+          const fs = require('fs');
+          const path = require('path');
+          const cliRoot = process.cwd();
+          const pkg = JSON.parse(fs.readFileSync(path.join(cliRoot, 'package.json'), 'utf8'));
+          const out = path.join(cliRoot, 'src/generated/pkg-info.ts');
+          fs.mkdirSync(path.dirname(out), { recursive: true });
+          fs.writeFileSync(out,
+            '// Generated by scripts/generate-pkg-info.ts - do not edit by hand.\n' +
+            'export const PKG_NAME = ' + JSON.stringify(pkg.name) + '\n' +
+            'export const PKG_VERSION = ' + JSON.stringify(pkg.version) + '\n');
+        "
+        exit 0
+        ;;
+      *)
+        echo "stub bun: unsupported run target: ${2:-}" >&2
+        exit 1
+        ;;
+    esac
+    ;;
+  *)
+    echo "stub bun: unsupported subcommand: ${1:-}" >&2
+    exit 1
+    ;;
+esac
+BUN_EOF
+  chmod +x "$repo/bin/bun"
+
+  # Stub `gh`: handles `auth status` (always 0) and `pr create` (logs args).
+  # Failure injection via GH_FAIL_AT="auth"|"pr-create".
+  cat >"$repo/bin/gh" <<'GH_EOF'
+#!/usr/bin/env bash
+set -euo pipefail
+
+if [[ -n "${GH_FAIL_AT:-}" ]]; then
+  if [[ "$GH_FAIL_AT" == "auth" && "${1:-}" == "auth" ]]; then
+    exit 1
+  fi
+  if [[ "$GH_FAIL_AT" == "pr-create" && "${1:-}" == "pr" && "${2:-}" == "create" ]]; then
+    echo "stub gh: forced PR create failure" >&2
+    exit 1
+  fi
+fi
+
+case "${1:-}" in
+  auth)
+    exit 0
+    ;;
+  pr)
+    if [[ "${2:-}" == "create" ]]; then
+      printf '%s\n' "$*" >> "${GH_LOG_FILE:-/dev/null}"
+      exit 0
+    fi
+    exit 1
+    ;;
+  *)
+    echo "stub gh: unsupported: ${1:-}" >&2
+    exit 1
+    ;;
+esac
+GH_EOF
+  chmod +x "$repo/bin/gh"
+
  cat >"$repo/.gitignore" <<EOF
 stdout.log
 stderr.log
-git-push-log.txt
+gh-stub.log
 bin-git/
 EOF

@ -108,7 +174,8 @@ EOF
  git -C "$repo" config user.name "Test User"
  git -C "$repo" config user.email "test@example.com"
  git -C "$repo" remote add origin "$origin"
-  git -C "$repo" add cli/package.json scripts/publish-cli.sh bin/npm .gitignore
+  git -C "$repo" add cli/package.json cli/src/generated/pkg-info.ts \
+                     scripts/publish-cli.sh bin/bun bin/gh .gitignore
  git -C "$repo" commit -q -m "init"
  git -C "$repo" push -q -u origin main

@ -119,24 +186,32 @@ EOF
  done
 }

-# run_publish <repo> <bump> [stdin]
-# Writes stdout to $repo/stdout.log and stderr to $repo/stderr.log.
-# Prints the exit code on stdout.
+# run_publish <repo> <bump> [stdin] [extra_env=...]
+#
+# Writes stdout/stderr to $repo/{stdout,stderr}.log and prints exit code.
+# `extra_env` (e.g. "BUN_FAIL_AT=lint") is forwarded as bash env assignments.
 run_publish() {
  local repo="$1"
  local bump="$2"
  local input="${3-}"
+  local extra="${4-}"
  local status=0
+  local cmd=(env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE
+    REPO_ROOT="$repo" PATH="$repo/bin:$PATH"
+    GH_LOG_FILE="$repo/gh-stub.log")
+  if [[ -n "$extra" ]]; then
+    # Split "K1=V1 K2=V2" into separate env entries.
+    local kv
+    for kv in $extra; do
+      cmd+=("$kv")
+    done
+  fi
+  cmd+=(bash "$repo/scripts/publish-cli.sh" "$bump")
+
  if [[ -n "$input" ]]; then
-    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=$?
+    printf '%s' "$input" | "${cmd[@]}" >"$repo/stdout.log" 2>"$repo/stderr.log" || status=$?
  else
-    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=$?
+    "${cmd[@]}" >"$repo/stdout.log" 2>"$repo/stderr.log" || status=$?
  fi
  echo "$status"
 }
@ -157,10 +232,9 @@ grep -F "Usage:" "$REPO1/stderr.log" >/dev/null
 echo "[test] dirty working tree aborts"
 REPO2="$(new_tmp)"
 init_repo "$REPO2"
-touch "$REPO2/dirty.txt"
+echo "junk" > "$REPO2/cli/package.json"
 status="$(run_publish "$REPO2" "patch")"
 [[ "$status" -ne 0 ]] || fail "expected non-zero exit for dirty tree"
-grep -F "checking git working tree" "$REPO2/stdout.log" >/dev/null
 grep -F "git working tree is not clean" "$REPO2/stderr.log" >/dev/null

 # ----------------------------------------------------------------------------
@ -176,149 +250,135 @@ grep -F "releases must be cut from 'main'" "$REPO3/stderr.log" >/dev/null
 grep -F "feature/x" "$REPO3/stderr.log" >/dev/null

 # ----------------------------------------------------------------------------
-# Test 4: package.json behind latest cli-v* tag → baseline sync, then bump
+# Test 4: gh auth fails → abort early
 # ----------------------------------------------------------------------------
-echo "[test] baseline sync from latest cli-v* tag, then bump + push"
+echo "[test] gh auth failure aborts"
 REPO4="$(new_tmp)"
-init_repo "$REPO4" "0.1.0" "cli-v0.2.0"
-status="$(run_publish "$REPO4" "patch" $'y\n')"
-[[ "$status" -eq 0 ]] || { cat "$REPO4/stderr.log" >&2; fail "expected success, got $status"; }
-grep -F "syncing package.json 0.1.0 -> 0.2.0 (from cli-v0.2.0)" "$REPO4/stdout.log" >/dev/null
-grep -F "bumping version (patch)" "$REPO4/stdout.log" >/dev/null
-grep -F "new version: 0.2.1 (tag: cli-v0.2.1)" "$REPO4/stdout.log" >/dev/null
-grep -F '"version": "0.2.1"' "$REPO4/cli/package.json" >/dev/null
-git -C "$REPO4" rev-parse "cli-v0.2.1" >/dev/null \
-  || fail "local tag cli-v0.2.1 missing"
-git -C "$REPO4" log --oneline | grep -F "chore(cli): bump version to 0.2.1" >/dev/null
-git -C "$REPO4.origin.git" rev-parse "cli-v0.2.1" >/dev/null \
-  || fail "origin tag cli-v0.2.1 missing — atomic push not delivered"
+init_repo "$REPO4"
+status="$(run_publish "$REPO4" "patch" "" "GH_FAIL_AT=auth")"
+[[ "$status" -ne 0 ]] || fail "expected non-zero exit when gh auth fails"
+grep -F "gh CLI is not authenticated" "$REPO4/stderr.log" >/dev/null

 # ----------------------------------------------------------------------------
-# Test 5: no cli-v* tags → fall back to package.json
+# Test 5: release branch already exists → abort before build (fail-fast)
 # ----------------------------------------------------------------------------
-echo "[test] no cli-v* tags falls back to package.json"
+echo "[test] existing release branch aborts before build"
 REPO5="$(new_tmp)"
-init_repo "$REPO5" "0.1.0"
-status="$(run_publish "$REPO5" "minor" $'y\n')"
-[[ "$status" -eq 0 ]] || { cat "$REPO5/stderr.log" >&2; fail "expected success, got $status"; }
-grep -F "no cli-v* tags found, bumping from package.json" "$REPO5/stdout.log" >/dev/null
-grep -F "new version: 0.2.0 (tag: cli-v0.2.0)" "$REPO5/stdout.log" >/dev/null
-git -C "$REPO5.origin.git" rev-parse "cli-v0.2.0" >/dev/null \
-  || fail "origin tag cli-v0.2.0 missing"
+init_repo "$REPO5" "0.1.0" "cli-v0.1.0"
+# Baseline is cli-v0.1.0, patch target is cli-v0.1.1, branch=release/cli-v0.1.1.
+# Create that branch locally so the pre-flight check catches it.
+git -C "$REPO5" branch "release/cli-v0.1.1"
+status="$(run_publish "$REPO5" "patch")"
+[[ "$status" -ne 0 ]] || fail "expected non-zero exit when release branch exists"
+grep -F "branch release/cli-v0.1.1 already exists locally" "$REPO5/stderr.log" >/dev/null \
+  || { cat "$REPO5/stderr.log" >&2; fail "expected 'branch already exists locally' error"; }

 # ----------------------------------------------------------------------------
-# Test 6: confirmation cancel → revert package.json, no commit, no tag
+# Test 6: confirmation cancel → no branch, no commit, no push
 # ----------------------------------------------------------------------------
-echo "[test] confirmation cancel reverts everything"
+echo "[test] confirmation cancel leaves no side effects"
 REPO6="$(new_tmp)"
 init_repo "$REPO6" "0.1.0" "cli-v0.1.0"
 INITIAL_HEAD="$(git -C "$REPO6" rev-parse HEAD)"
 status="$(run_publish "$REPO6" "patch" $'n\n')"
 [[ "$status" -ne 0 ]] || fail "expected non-zero exit on cancel"
 grep -F "release cancelled" "$REPO6/stderr.log" >/dev/null
-grep -F '"version": "0.1.0"' "$REPO6/cli/package.json" >/dev/null \
-  || fail "package.json not reverted to 0.1.0 after cancel"
 [[ "$(git -C "$REPO6" rev-parse HEAD)" == "$INITIAL_HEAD" ]] \
-  || fail "HEAD advanced after cancel — extra commit was made"
-if git -C "$REPO6" rev-parse -q --verify "refs/tags/cli-v0.1.1" >/dev/null 2>&1; then
-  fail "tag cli-v0.1.1 must not exist after cancel"
+  || fail "HEAD advanced after cancel"
+[[ "$(git -C "$REPO6" rev-parse --abbrev-ref HEAD)" == "main" ]] \
+  || fail "not on main after cancel"
+if git -C "$REPO6" rev-parse -q --verify "refs/heads/release/cli-v0.1.1" >/dev/null 2>&1; then
+  fail "release branch must not exist after cancel"
 fi
 [[ -z "$(git -C "$REPO6" status --porcelain)" ]] \
-  || fail "working tree not clean after cancel — revert incomplete"
+  || fail "working tree not clean after cancel"

 # ----------------------------------------------------------------------------
-# Test 7: happy path — atomic push delivers branch + tag together
+# Test 7: happy path — branch pushed, PR opened, returned to main
 # ----------------------------------------------------------------------------
-echo "[test] happy path pushes branch and tag atomically"
+echo "[test] happy path pushes branch and opens PR"
 REPO7="$(new_tmp)"
 init_repo "$REPO7" "0.5.0"
 status="$(run_publish "$REPO7" "patch" $'y\n')"
 [[ "$status" -eq 0 ]] || { cat "$REPO7/stderr.log" >&2; fail "expected success, got $status"; }
-ORIGIN7="$REPO7.origin.git"
-git -C "$ORIGIN7" rev-parse "cli-v0.5.1" >/dev/null \
-  || fail "origin missing tag cli-v0.5.1"
-ORIGIN_HEAD="$(git -C "$ORIGIN7" rev-parse main)"
-LOCAL_HEAD="$(git -C "$REPO7" rev-parse main)"
-[[ "$ORIGIN_HEAD" == "$LOCAL_HEAD" ]] \
-  || fail "origin/main HEAD did not advance to match local main"
-TAG_COMMIT="$(git -C "$ORIGIN7" rev-parse "cli-v0.5.1^{commit}")"
-[[ "$TAG_COMMIT" == "$ORIGIN_HEAD" ]] \
-  || fail "origin tag cli-v0.5.1 does not point to origin/main HEAD"
-grep -F "release triggered" "$REPO7/stdout.log" >/dev/null

-# ----------------------------------------------------------------------------
-# Test 8: push failure → script exits non-zero (commit + tag stay local)
-#
-# Use a git wrapper that fails only on `git push`, so pull/fetch succeed
-# but the final push does not.
-# ----------------------------------------------------------------------------
-echo "[test] push failure surfaces error"
-REPO8="$(new_tmp)"
-init_repo "$REPO8" "0.6.0"
-mkdir -p "$REPO8/bin-git"
-cat >"$REPO8/bin-git/git" <<'WRAPPER'
-#!/usr/bin/env bash
-if [[ "$*" == *"push"* ]]; then
-  echo "fatal: could not read from remote repository." >&2
-  exit 128
+# Returned to main with branch deleted locally.
+[[ "$(git -C "$REPO7" rev-parse --abbrev-ref HEAD)" == "main" ]] \
+  || fail "not back on main after success"
+if git -C "$REPO7" rev-parse -q --verify "refs/heads/release/cli-v0.5.1" >/dev/null 2>&1; then
+  fail "local release branch should be deleted after success"
 fi
-exec /usr/bin/git "$@"
-WRAPPER
-chmod +x "$REPO8/bin-git/git"
-status=0
-printf 'y\n' | env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE \
-  REPO_ROOT="$REPO8" PATH="$REPO8/bin-git:$REPO8/bin:$PATH" \
-  bash "$REPO8/scripts/publish-cli.sh" "patch" \
-  >"$REPO8/stdout.log" 2>"$REPO8/stderr.log" || status=$?
-[[ "$status" -ne 0 ]] || fail "expected non-zero exit when push fails"
-git -C "$REPO8" rev-parse "cli-v0.6.1" >/dev/null \
-  || fail "local tag cli-v0.6.1 missing after push failure"
-git -C "$REPO8" log --oneline | grep -F "chore(cli): bump version to 0.6.1" >/dev/null \
-  || fail "local bump commit missing after push failure"
+
+# Branch on origin with bump commit.
+ORIGIN7="$REPO7.origin.git"
+git -C "$ORIGIN7" rev-parse "refs/heads/release/cli-v0.5.1" >/dev/null \
+  || fail "origin missing release branch"
+TIP_MSG="$(git -C "$ORIGIN7" log -1 --format=%s "refs/heads/release/cli-v0.5.1")"
+[[ "$TIP_MSG" == "chore(cli): bump version to 0.5.1" ]] \
+  || fail "unexpected commit message on release branch: $TIP_MSG"
+
+# pkg-info.ts AND package.json both updated in the bump commit.
+git -C "$ORIGIN7" show "refs/heads/release/cli-v0.5.1:cli/package.json" \
+  | grep -F '"version": "0.5.1"' >/dev/null \
+  || fail "package.json on branch missing new version"
+git -C "$ORIGIN7" show "refs/heads/release/cli-v0.5.1:cli/src/generated/pkg-info.ts" \
+  | grep -F 'PKG_VERSION = "0.5.1"' >/dev/null \
+  || fail "pkg-info.ts on branch missing new version"
+
+# gh pr create was invoked with expected args.
+[[ -f "$REPO7/gh-stub.log" ]] || fail "gh stub log missing"
+grep -F "pr create" "$REPO7/gh-stub.log" >/dev/null \
+  || fail "gh pr create not invoked"
+grep -F -- "--head release/cli-v0.5.1" "$REPO7/gh-stub.log" >/dev/null \
+  || fail "gh pr create missing --head"
+grep -F -- "--base main" "$REPO7/gh-stub.log" >/dev/null \
+  || fail "gh pr create missing --base"

 # ----------------------------------------------------------------------------
-# Test 9: 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.
+# Test 8: baseline from latest cli-v* tag (not package.json)
 # ----------------------------------------------------------------------------
-echo "[test] unpushed detection catches tag-only failure"
+echo "[test] baseline taken from latest cli-v* tag"
+REPO8="$(new_tmp)"
+init_repo "$REPO8" "0.1.0" "cli-v0.2.0"
+status="$(run_publish "$REPO8" "patch" $'y\n')"
+[[ "$status" -eq 0 ]] || { cat "$REPO8/stderr.log" >&2; fail "expected success, got $status"; }
+grep -F "baseline: 0.2.0 (from cli-v0.2.0)" "$REPO8/stdout.log" >/dev/null \
+  || fail "baseline log missing — version computed from wrong source"
+git -C "$REPO8.origin.git" rev-parse "refs/heads/release/cli-v0.2.1" >/dev/null \
+  || fail "expected branch release/cli-v0.2.1 on origin"
+
+# ----------------------------------------------------------------------------
+# Test 9: gh pr create fails → branch stays on origin, recovery printed
+# ----------------------------------------------------------------------------
+echo "[test] gh pr create failure triggers pushed-stage cleanup"
 REPO9="$(new_tmp)"
-init_repo "$REPO9" "0.7.0"
-cd "$REPO9/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 "$REPO9"
-git -C "$REPO9" add cli/package.json
-git -C "$REPO9" commit -q -m "chore(cli): bump version to 0.7.1"
-git -C "$REPO9" tag "cli-v0.7.1"
-git -C "$REPO9" push -q origin main
-# Tag NOT pushed — simulates atomic push partial failure recovery
-status="$(run_publish "$REPO9" "patch")"
-[[ "$status" -ne 0 ]] || fail "expected non-zero exit when unpushed tag detected"
-grep -F "Unpushed tags" "$REPO9/stderr.log" >/dev/null \
-  || { cat "$REPO9/stderr.log" >&2; fail "expected unpushed tag warning"; }
-grep -F "cli-v0.7.1" "$REPO9/stderr.log" >/dev/null \
-  || fail "expected cli-v0.7.1 in unpushed tag warning"
+init_repo "$REPO9" "0.6.0"
+status="$(run_publish "$REPO9" "patch" $'y\n' "GH_FAIL_AT=pr-create")"
+[[ "$status" -ne 0 ]] || fail "expected non-zero exit when gh pr create fails"
+
+# Recovery instructions emitted by the trap.
+grep -F "release branch 'release/cli-v0.6.1' was pushed but PR creation failed" \
+  "$REPO9/stderr.log" >/dev/null \
+  || { cat "$REPO9/stderr.log" >&2; fail "missing pushed-stage recovery message"; }
+grep -F "git push origin --delete release/cli-v0.6.1" "$REPO9/stderr.log" >/dev/null \
+  || fail "missing rollback hint in recovery message"
+
+# Branch is on origin (push succeeded before gh failed).
+git -C "$REPO9.origin.git" rev-parse "refs/heads/release/cli-v0.6.1" >/dev/null \
+  || fail "expected branch on origin after push, before failed PR"

 # ----------------------------------------------------------------------------
-# Test 10: --atomic flag is actually passed to git push
-#
-# Use a git wrapper to capture the push command and verify --atomic is present.
+# Test 10: push fails → committed-stage cleanup, branch deleted locally
 # ----------------------------------------------------------------------------
-echo "[test] push uses --atomic flag"
+echo "[test] push failure rolls back local branch"
 REPO10="$(new_tmp)"
-init_repo "$REPO10" "0.8.0"
+init_repo "$REPO10" "0.7.0"
 mkdir -p "$REPO10/bin-git"
 cat >"$REPO10/bin-git/git" <<'WRAPPER'
 #!/usr/bin/env bash
-if [[ "$*" == *"push"* ]]; then
-  echo "GIT_PUSH_ARGS: $*" >> "$REPO_ROOT/git-push-log.txt"
+if [[ "${1:-}" == "-C" && "${3:-}" == "push" ]]; then
+  echo "fatal: forced push failure" >&2
+  exit 128
 fi
 exec /usr/bin/git "$@"
 WRAPPER
@ -326,10 +386,29 @@ chmod +x "$REPO10/bin-git/git"
 status=0
 printf 'y\n' | env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE \
  REPO_ROOT="$REPO10" PATH="$REPO10/bin-git:$REPO10/bin:$PATH" \
+  GH_LOG_FILE="$REPO10/gh-stub.log" \
  bash "$REPO10/scripts/publish-cli.sh" "patch" \
  >"$REPO10/stdout.log" 2>"$REPO10/stderr.log" || status=$?
-[[ "$status" -eq 0 ]] || { cat "$REPO10/stderr.log" >&2; fail "expected success, got $status"; }
-grep -F -- "--atomic" "$REPO10/git-push-log.txt" >/dev/null \
-  || { cat "$REPO10/git-push-log.txt" >&2; fail "git push did not include --atomic flag"; }
+[[ "$status" -ne 0 ]] || fail "expected non-zero exit when push fails"
+
+grep -F "rolling back local branch" "$REPO10/stderr.log" >/dev/null \
+  || { cat "$REPO10/stderr.log" >&2; fail "missing committed-stage rollback message"; }
+
+# Back on main, release branch deleted.
+[[ "$(git -C "$REPO10" rev-parse --abbrev-ref HEAD)" == "main" ]] \
+  || fail "not on main after push failure cleanup"
+if git -C "$REPO10" rev-parse -q --verify "refs/heads/release/cli-v0.7.1" >/dev/null 2>&1; then
+  fail "local release branch should be deleted after push failure"
+fi
+
+# Origin must NOT have the branch (push was blocked).
+if git -C "$REPO10.origin.git" rev-parse -q --verify "refs/heads/release/cli-v0.7.1" >/dev/null 2>&1; then
+  fail "origin should not have branch when push failed"
+fi
+
+# gh pr create must NOT have run.
+if [[ -f "$REPO10/gh-stub.log" ]] && grep -F "pr create" "$REPO10/gh-stub.log" >/dev/null; then
+  fail "gh pr create ran despite push failure"
+fi

 echo "all tests passed"