Skip to content

refactor(cli): improve publish-cli script reliability#441

Merged
dongmucat merged 5 commits into
mainfrom
feat/cli-auto-build
Jun 2, 2026
Merged

refactor(cli): improve publish-cli script reliability#441
dongmucat merged 5 commits into
mainfrom
feat/cli-auto-build

Conversation

@Pipreola

Copy link
Copy Markdown
Collaborator

Summary

优化 publish-cli.sh 脚本的可靠性和用户体验:

  • 预检查前移:将版本计算和 tag/branch 冲突检测移到 build-and-test 之前执行,避免在发现冲突前浪费几分钟跑构建
  • 信号处理增强:trap 现在捕获 INT/TERM 信号(Ctrl+C),确保中断时正确清理工作树状态
  • 更新 Makefile 帮助文本:反映基于 PR 的发布流程

改进细节

1. 预检查前移(fail-fast)

之前git pullbun install / lint / typecheck / test / build(几分钟)→ 计算版本 → 检查 tag/branch 是否存在

现在git pull → 计算版本 → 检查 tag/branch 是否存在 → bun install / lint / typecheck / test / build

如果 release/cli-vX.Y.Z 分支或 tag 已存在(比如上次 PR 已合并但还没打 tag),现在会在几秒内报错退出,而不是白跑几分钟构建。

2. 信号处理增强

之前trap cleanup_on_error ERR — 只捕获命令失败

现在trap cleanup_on_error ERR INT TERM — 同时捕获 Ctrl+C 和 kill 信号

用户在 bun testbun build 阶段按 Ctrl+C 时,cleanup 函数会:

  • 恢复被 pre-* hook 弄脏的 cli/src/generated/pkg-info.ts
  • 确保以非零退出码退出(而不是误报成功)

Test plan

  • 语法检查通过(bash -n scripts/publish-cli.sh
  • 模拟冲突场景:手动创建 release/cli-vX.Y.Z 分支,验证脚本在 build 前就报错
  • 模拟中断场景:在 bun test 阶段按 Ctrl+C,验证工作树被正确恢复
  • 正常发布流程:跑一次完整的 make publish-cli patch(或在测试环境)

相关 PR

这是 #422 的后续优化,基于 code review 反馈改进脚本健壮性。

- Move version computation and pre-flight checks before build-and-test
  to fail fast on conflicts (existing branch/tag) instead of wasting
  minutes on lint/test/build
- Add INT/TERM signal handlers to cleanup trap so Ctrl+C during build
  properly restores working tree state
- Update Makefile help text to reflect PR-based workflow
gemini-code-assist[bot]

This comment was marked as outdated.

Pipreola added 2 commits May 15, 2026 13:42
Address code review feedback from gemini-code-assist bot:

- Use `git checkout -f` in on-release and committed cleanup stages
  to ensure reliable branch switching even when files are staged
  but not committed (e.g., interrupted after `git add` but before
  `git commit`)
- Remove redundant `git checkout -- <file>` in on-release stage
  since `-f` already discards all local changes

This prevents cleanup failures when the script is interrupted
between staging and committing.
- 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)
@Pipreola

Copy link
Copy Markdown
Collaborator Author

Review findings addressed in efcf0ee

# Issue Resolution
Blocker 1 scripts/tests/publish-cli-test.sh all broken Fully rewritten — 10 tests covering new flow, all passing
Major 2 cli/RELEASE.md describes old tag-push flow Rewritten to document PR-based flow
Major 3 Commit message contains AI tool name Skipped — commit already on main, rewriting public history too costly
Major 4 Node -e command injection via string interpolation Fixed — all 3 node calls now use process.env
Bug if ! gh pr create bypasses ERR trap Fixed — direct call, set -e triggers trap correctly

Minor items 5/6/9 (EXIT trap, stderr suppression, trap cleanup inconsistency) are deferred — current code is safe because CLEANUP_STAGE=pre-branch at all explicit exit 1 points before branching.

Tests run:

$ bash scripts/tests/publish-cli-test.sh
[test] invalid bump type
[test] dirty working tree aborts
[test] non-main branch aborts
[test] gh auth failure aborts
[test] existing release branch aborts before build
[test] confirmation cancel leaves no side effects
[test] happy path pushes branch and opens PR
[test] baseline taken from latest cli-v* tag
[test] gh pr create failure triggers pushed-stage cleanup
[test] push failure rolls back local branch
all tests passed

@dongmucat

Copy link
Copy Markdown
Collaborator

我重新看了最新 head(efcf0ee19d6d0235060e6efc6fa2ed20475b073f)。脚本语法、git diff --check、以及新增的 scripts/tests/publish-cli-test.sh 本身都能通过,但这里还有几个发布流程风险,建议合并前修掉:

  1. 合并后的手动 tag 没有绑定到 main / origin/main 的具体提交。

scripts/publish-cli.sh 生成的 PR body 和最终提示,以及 cli/RELEASE.md,目前都是:

git pull origin main
git tag cli-vX.Y.Z
git push origin cli-vX.Y.Z

git pull origin main 会作用在当前分支,git tag 也会标记当前 HEAD。如果操作者不在 main,或者本地 main 有本地 merge/diverge,就可能把 release tag 打到错误提交上,而 release-cli.yml 会直接基于这个 tag 发布。建议明确 fetch 后把 tag 绑定到 origin/main 或已验证的 merge commit,例如先校验/切到干净的 main,再 git tag cli-vX.Y.Z origin/main

  1. prerelease tag 与脚本版本计算规则不一致。

release-cli.yml 允许 cli-vX.Y.Z-prerelease,但 scripts/publish-cli.sh 从最新 cli-v* tag 得到 BASE_VERSION 后用 split('.') + Number 只接受纯 X.Y.Z。如果最新合法 tag 是 cli-v0.2.0-rc.1,后续 make publish-cli 会直接在版本计算阶段失败。这里要么脚本跳过 prerelease tag/正确解析 semver,要么 workflow 收窄 tag 规则,并补对应测试。

  1. 新增的发布脚本集成测试没有进入 PR CI gate。

publish-cli-test.sh 覆盖了新 release branch + PR flow、PR 创建失败、push 失败等场景,但当前 PR CLI workflow 只跑 cli/** 的 install/lint/typecheck/test/build,没有执行这个脚本,触发路径也不包含 scripts/**。这样 release 脚本回归仍可能在绿色 CI 下合入。建议把 scripts/tests/publish-cli-test.sh 纳入相关 PR workflow,并让 scripts/** 变更能触发它。

- Bind release tag to origin/main: PR body, end-of-run hint, and
  cli/RELEASE.md now use `git tag $TAG origin/main` so the tag is
  always placed on the merged commit, regardless of local branch state
- Reject prerelease tags in version computation: if the latest cli-v*
  tag contains non-X.Y.Z characters (e.g., -rc.1), exit with a clear
  message instead of crashing in node parsing
- Add pr-scripts.yml workflow: runs publish-cli-test.sh on scripts/**
  changes so the release script regression suite gates PRs
- Add Test 11 covering prerelease tag rejection
@Pipreola

Copy link
Copy Markdown
Collaborator Author

All three addressed in 7847e0a:

  1. Tag now binds to origin/main explicitly — PR body, script hint, and RELEASE.md all updated to git tag $TAG origin/main.
  2. Prerelease tags rejected early — if the latest cli-v* tag has non-X.Y.Z chars, script exits with a clear message. Test 11 covers this.
  3. Added pr-scripts.yml — triggers on scripts/** changes, runs publish-cli-test.sh on ubuntu. Release script regressions now gate PRs.

@dongmucat

Copy link
Copy Markdown
Collaborator

这里还有一个发布流程的 blocker:scripts/publish-cli.sh 现在用本地 tag 集合来计算版本基线。fetch --tags --prune 之后的 git tag --list 'cli-v*' --sort=-version:refname | head -n1 仍然会包含本地孤儿 tag,但脚本没有验证这个 baseline tag 是否真的存在于 origin

一个实际失败路径是:合并 release PR 后,按文档先执行 git tag cli-vX.Y.Z origin/main,但后续 git push origin cli-vX.Y.Z 因网络/权限失败。此时本地会留下一个 origin 上不存在的 cli-v* tag。下一次运行 make publish-cli 会把这个未发布 tag 当作最新版本,生成下一个 release branch,导致跳版本或基于未发布版本继续发版。

建议把版本基线改成只从远端 tag 计算,或者恢复/新增“本地 cli-v* tag 不存在于 origin 就 fail”的检测,并补一个 local-only tag 的测试用例。

A failed `git push origin cli-vX.Y.Z` after a successful local tag
leaves an orphan tag locally. The previous `git tag --list` baseline
would then treat it as the latest release, causing skipped versions or
publishes based on an unreleased tag.

Switch to `git ls-remote --tags --refs origin 'cli-v*' | sort -V` so
the baseline reflects only what is actually on origin. Local orphan
tags can still collide with the computed target tag, which fails fast
with a clear message as before.

Adds test 12 covering the orphan-tag scenario.
@Pipreola

Copy link
Copy Markdown
Collaborator Author

已修复,commit 239cd2e

把基线计算从 git tag --list 'cli-v*' --sort=-version:refname 改成 git ls-remote --tags --refs origin 'cli-v*' | sort -V | tail -n1,基线只从远端 tag 计算,本地孤儿 tag 不再影响版本。

同时移除了之前的 git fetch --tags --prune origin——既然基线只看远端,没必要再把 tag 拉到本地。本地孤儿 tag 仍会被「目标 tag 已存在」检查捕获,行为不变。

补了 Test 12 覆盖 local-only orphan:origin 上 cli-v0.1.0,本地再加一个孤儿 cli-v0.3.0,run make publish-cli patch 应该算出 0.1.1(而不是 0.3.1)。

$ bash scripts/tests/publish-cli-test.sh
...
[test] prerelease tag aborts with message
[test] local-only orphan tag ignored for baseline
all tests passed

@dongmucat dongmucat merged commit 0b1c366 into main Jun 2, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants