Skip to content

fix(self-update): guard against non-master branch and dirty tree#438

Merged
icebear0828 merged 1 commit intodevfrom
fix/self-update-safety-guards
May 4, 2026
Merged

fix(self-update): guard against non-master branch and dirty tree#438
icebear0828 merged 1 commit intodevfrom
fix/self-update-safety-guards

Conversation

@icebear0828
Copy link
Copy Markdown
Owner

Summary

applyProxySelfUpdate 之前会无条件 git checkout -- . + git pull origin master。两个真实后果:dev 服跑着改代码时,tsx watch 每次重启触发 10s update check,开发者刚保存的改动会在 ~12s 内被 git checkout -- . 静默吞掉;并且即使在 dev 等非 master 分支上也照样把 master 合进来,破坏 dev→master promote 流程。本 PR 加双重护栏 + 移除危险的 checkout 调用。

Changes

  • src/self-update.tsapplyProxySelfUpdate 进入即先校验
    • 当前分支必须是 master / main,否则 abort 返回错误
    • git status --porcelain 必须为空,否则 abort 返回错误
    • 删除 原本的 git checkout -- .;干净 master 上的 git pull 已足够
  • tests/unit/self-update.test.ts:4 条新 case(干净 master 走完整流程并断言无 checkout 调用、dev 分支被拒、main 分支被接受、脏树被拒)
  • tests/e2e/self-update.test.ts:5 条 mock 序列改为 [branch, status, ...] 适配新调用顺序
  • CHANGELOG.md[Unreleased] → Fixed 第一条

Test Plan

  • npx vitest run tests/unit/self-update.test.ts — 28 通过
  • npx vitest run tests/e2e/self-update.test.ts — 15 通过 + 1 skipped
  • npm test — 1657 通过 + 1 skipped
  • npx tsc --noEmit — 无报错
  • pre-push hook — 通过

Notes

发现路径有点意思:起初是排查"开 dev server 改代码会被神秘覆盖",靠 sudo fs_usage 抓到 git.<pid> 在写源码文件,回溯到 src/self-update.ts:458git checkout -- .。所有跑在 dev 分支并启用 auto_update: true 的开发者都会中招。

applyProxySelfUpdate previously ran `git checkout -- .` followed by
`git pull origin master` unconditionally. Two real consequences:

1. Any uncommitted local change was silently discarded. With dev server
   running under tsx watch, every restart triggered the 10s update
   check; if auto_update was on, the developer's in-flight edits were
   wiped within ~12s of save.
2. The pull merged master into whatever branch was checked out — on
   `dev` this corrupts the dev→master promote contract documented in
   CLAUDE.md.

Add two safety guards before any destructive op:

- Refuse to apply when current branch is not `master` / `main`.
- Refuse to apply when `git status --porcelain` is non-empty.

Drop the silent `git checkout -- .` entirely; if the tree is clean and
on master, plain `git pull` is sufficient. Both refusals return the
existing error envelope so the dashboard / API can surface the reason.
@icebear0828 icebear0828 merged commit 6b7bb46 into dev May 4, 2026
1 check passed
@icebear0828 icebear0828 deleted the fix/self-update-safety-guards branch May 4, 2026 02:04
icebear0828 added a commit that referenced this pull request May 5, 2026
The soak check measures `now - dev_HEAD_timestamp >= 24h`, which means
every new merge into dev resets the clock. Under any non-trivial merge
cadence, dev never satisfies the soak gate and master stagnates: PRs
#437/#438/#439/#440/#442 all stacked on dev for a week with no
promotion.

Add a `force_skip_soak` boolean input to workflow_dispatch (default
false). Schedule cron remains untouched and continues to enforce the
24h rule. Only manual triggers can bypass, and only when the operator
explicitly sets the input to true — intended for sync-back / merge
commits whose content has actually been on dev long enough but whose
HEAD timestamp is misleadingly fresh.

Test plan: yaml syntax verified via js-yaml. Functional verification
will be the next manual workflow_dispatch run with the input set.

Co-authored-by: icebear0828 <icebear0828@users.noreply.github.com>
icebear0828 added a commit that referenced this pull request May 5, 2026
applyProxySelfUpdate previously ran `git checkout -- .` followed by
`git pull origin master` unconditionally. Two real consequences:

1. Any uncommitted local change was silently discarded. With dev server
   running under tsx watch, every restart triggered the 10s update
   check; if auto_update was on, the developer's in-flight edits were
   wiped within ~12s of save.
2. The pull merged master into whatever branch was checked out — on
   `dev` this corrupts the dev→master promote contract documented in
   CLAUDE.md.

Add two safety guards before any destructive op:

- Refuse to apply when current branch is not `master` / `main`.
- Refuse to apply when `git status --porcelain` is non-empty.

Drop the silent `git checkout -- .` entirely; if the tree is clean and
on master, plain `git pull` is sufficient. Both refusals return the
existing error envelope so the dashboard / API can surface the reason.

Co-authored-by: icebear0828 <icebear0828@users.noreply.github.com>
icebear0828 added a commit that referenced this pull request May 5, 2026
The soak check measures `now - dev_HEAD_timestamp >= 24h`, which means
every new merge into dev resets the clock. Under any non-trivial merge
cadence, dev never satisfies the soak gate and master stagnates: PRs
#437/#438/#439/#440/#442 all stacked on dev for a week with no
promotion.

Add a `force_skip_soak` boolean input to workflow_dispatch (default
false). Schedule cron remains untouched and continues to enforce the
24h rule. Only manual triggers can bypass, and only when the operator
explicitly sets the input to true — intended for sync-back / merge
commits whose content has actually been on dev long enough but whose
HEAD timestamp is misleadingly fresh.

Test plan: yaml syntax verified via js-yaml. Functional verification
will be the next manual workflow_dispatch run with the input set.

Co-authored-by: icebear0828 <icebear0828@users.noreply.github.com>
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.

1 participant