From e2ca9732af9f233a0c1dfefdcc28c77fddf6fa6f Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Wed, 5 Aug 2026 14:20:59 +0700 Subject: [PATCH 01/12] =?UTF-8?q?feat(pilot):=20=E5=81=9A=E6=8E=89=20FU-1/?= =?UTF-8?q?FU-2/FU-4=20=E2=80=94=E2=80=94=20flag=20=E6=94=B9=E7=99=BD?= =?UTF-8?q?=E5=90=8D=E5=8D=95=20+=20safe-cleanup=20=E6=94=AF=E6=8C=81=20sq?= =?UTF-8?q?uash=20=E4=BB=93=E5=BA=93?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## FU-1:merge-pr 的 gh flag 从黑名单改成白名单 黑名单只能拒绝**今天存在**的危险 flag。哪天 `gh pr merge` 新增一个能绕过分支保护的 flag, 黑名单会静默放行,这里什么都不会察觉。白名单朝相反方向失败:不认识的 flag 一律拒绝, 直到有人**刻意**把它加进去 —— 这才是护栏该有的失败方向。 放行:`--squash --merge --rebase --auto`,以及带值的 `--body/-b --body-file/-F --subject/-t --match-head-commit`(分离式与 `=` 附着式都支持)。 `--admin` / `--repo` / `-d` 保留各自的专门错误消息(白名单本来也会拒,但那样说不清为什么)。 ## FU-2 + FU-4:safe-cleanup 支持 squash-merge 仓库 squash 仓库里 `git branch --merged` **恒返回 0** —— squash 重写补丁,原 commit 不是集成分支的 祖先。实测本仓库 28 个分支返回 0,于是这个脚本在自己家里什么都清理不了,人只能手工 `-D`, 比脚本存在还糟。 新增 `--squash-merged`(opt-in),引入第二种**服务端**证据:GitHub 的 `GET /repos/{o}/{r}/commits/{sha}/pulls` 回答「哪个 PR 把这个 commit 引入了仓库」, 只有拿到 `merged_at != null` 的 PR 才允许 `-D`。 **实现没用 FU-4 记的祖先算法,用了更好的**:祖先算法要先把所有已合并 PR 的 head 抓到本地 再算可达性;这个接口每个分支只要一次 API 调用、不写任何 ref 或对象 —— **dry-run 因此保持零写入**。 两者同样是按 commit 判而不是按分支名判,所以两个方向的错都不存在: - 漏删:`work-pr18` 这类分支名从没当过 PR head,但 tip 就是别的 PR 的已合并 head - 误删:分支名可复用,同名分支删掉重开后内容全不同,旧的 MERGED PR 仍然匹配名字 四种形状实测:squash 后的 head ✓ / 分支中间的 commit ✓ / CLOSED 未合并 → 无证据 ✓ / 从未开过 PR → 无证据 ✓。 ## 三轮对抗自审抓到的一条(不在计划里) **「查不了」曾经和「没有可清理的」长得一模一样。** `merged_pr_for` 只在 `$( )` 里被调用, 那是**子 shell**,它设的 `gh_state` 返回后就丢了 —— 于是「无法核实」那条分支永远不可达, gh 没装/没登录时打印的是 `(none)`,读起来就是「干净,没东西可清」。改成在主 shell 里先 `gh_init` 探测一次。这正是「沉默不等于成功」那个坑,而且是我自己写出来的。 ## 六项实测 1. 基线 dry-run → `(none)`(本地仅剩不该删的两个,无证据) 2. 无 flag → 列为 `candidate ... — pass --squash-merged to include` 3. `--squash-merged` 无 `--apply` → `would delete`,分支仍在 4. `--squash-merged --apply` → 只删有证据的;无证据的原样保留 5. **gh 不可用**(PATH 里藏掉 gh)→ `(cannot verify … branches KEPT)`,不删任何东西 6. 受保护分支(`release/_test`,指向已合并 commit)→ **不入列**,计数为 0 白名单侧另测六种形状:`--squash` 放行 / `--admin` 专门消息 / `--delete-branch` 专门消息 / 未知 flag 报「这是白名单」并列出允许项 / `--body` 缺值报错 / 多余位置参数被拒。 README、SKILL.md、reference/git-safety.md 里「永不 -D」的表述同步改准 —— 那是本次唯一放宽的 保证,不能只改代码不改承诺。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- docs/agent/followups.md | 7 ++ plugins/pilot/skills/pilot/README.md | 3 +- plugins/pilot/skills/pilot/SKILL.md | 2 +- .../skills/pilot/reference/git-safety.md | 12 +- .../pilot/skills/pilot/scripts/git-guard.sh | 37 ++++-- .../skills/pilot/scripts/safe-cleanup.sh | 115 +++++++++++++++++- 6 files changed, 157 insertions(+), 19 deletions(-) diff --git a/docs/agent/followups.md b/docs/agent/followups.md index 4ee077e..33e907d 100644 --- a/docs/agent/followups.md +++ b/docs/agent/followups.md @@ -14,6 +14,13 @@ > > **以 FU-4 的判据为准**：本地 tip **==** 或 **是任何一个**已合并 PR 的 `headRefOid` 的祖先 > （先 `git fetch origin pull/N/head` 把 head 抓到本地再算祖先）。这一条同时挡住上面两种错。 +> +> **⚠️ 再更正（实现时找到更好的）**：FU-4 的祖先算法是对的，但**实现用的不是它**。 +> GitHub 有 `GET /repos/{owner}/{repo}/commits/{sha}/pulls`，直接回答「哪个 PR 把这个 commit +> 引入了仓库」。它同样按 commit 判、两种错都没有，而且**每个分支只要一次 API 调用**、 +> 不用把所有 PR head 抓到本地 —— dry-run 因此保持零写入。四种形状实测通过： +> squash 后的 head ✓、分支中间的 commit ✓、CLOSED 未合并 → 无证据 ✓、从未开过 PR → 无证据 ✓。 +> 落地在 `safe-cleanup.sh` 的 `merged_pr_for()`。 - [ ] FU-1 · B · src=PR#39 review [Low] · 2026-08-05 · git-guard merge-pr 拒绝 gh flag 用的是黑名单(--admin/--repo/-R 及其粘连形式)。黑名单追不上新 flag —— 以后 gh pr merge 若新增能绕过分支保护的 flag,这里不会自动知道。改成白名单(只放行 --squash/--merge/--rebase 等已知安全 flag)更耐久 - [ ] FU-2 · B · src=2026-08-05 合并 #38/#39/#40/#41 后实测 · 2026-08-05 · safe-cleanup.sh 在 squash-merge 仓库里永远清不掉任何分支:本仓库 28 个本地分支,git branch --merged main 返回 0 个,因为 squash 后原 commit 不是 main 的祖先,而 safe-cleanup 只用 -d 永不 -D。这是继 #39(死代码)、#40(随机红灯)之后同一家族的第三个『守卫跑不起来』。正确改法:用 gh 核实『存在 headRefName==该分支且 state==MERGED 的 PR』作为已合并证据,再允许 -D;不能简单放开 -D diff --git a/plugins/pilot/skills/pilot/README.md b/plugins/pilot/skills/pilot/README.md index 737255e..166ca32 100644 --- a/plugins/pilot/skills/pilot/README.md +++ b/plugins/pilot/skills/pilot/README.md @@ -16,7 +16,8 @@ pilot doctor # 自检本地就绪度（config/docs/分支/gh/hook）——** ## 为什么用它 把「有经验程序员的默认」固化成流程，且**危险动作确定性可控**： -- **安全清理**：只删「已合并进集成分支 + 干净」的分支/worktree；只 `git branch -d`（永不 `-D`）；默认 dry-run，`--apply` 才动手；护住主干/集成/当前/protected/脏 worktree。逻辑全在 `scripts/safe-cleanup.sh`，不靠模型临场判断。 +- **安全清理**：只删「已合并进集成分支 + 干净」的分支/worktree；默认只 `git branch -d`；默认 dry-run，`--apply` 才动手；护住主干/集成/当前/protected/脏 worktree。逻辑全在 `scripts/safe-cleanup.sh`，不靠模型临场判断。 + **squash-merge 仓库**里 `git branch --merged` 恒返回 0（squash 重写补丁，原 commit 不是集成分支的祖先），此时加 `--squash-merged`：它按 **commit** 向 GitHub 查「哪个已合并 PR 引入了这个提交」，拿到证据才用 `-D`；查不到证据、没装 gh、没登录 —— 一律保留。 - **PR 纪律**：绝不 `git add -A`；绝不直推/直合主干；一个 task = 一个分支 = 一个 worktree = 一个 PR；PR 前必自测 + 对抗式 review。 - **外部 review 回路（已生产验证）**：pilot **不自评 PR**——开好 PR 后只盯自己 PR 的状态（`scripts/pr-monitor.sh --pr --wait-for-verdict`，内置 3–5 分钟轮询与 **30 分钟硬上限**；只有评审 commit == 当前 head 才算裁决）。裁决由**外部评审服务**给出，契约见 `reference/review-contract.md`：排队 5–10 分钟、评审 5–10 分钟，通常 20 分钟内出 `APPROVED`/`CHANGES_REQUESTED`（超大 PR 例外）。推新 commit 自动触发再评审。**那个服务是什么、装在哪、覆盖哪些仓库，pilot 一概不知也不启动**——只依赖这份契约。 - **pilot 是入口，配套能力由它安排**：飞书 / Notion 等文档源是 pilot 的**配套 skill**——该不该装、装哪个、装到全局还是项目级、装完怎么验证，由 pilot 负责讲清楚和安排。**但「入口」是编排责任，不是运行时依赖**：pilot 自己不 import、不启动任何文档源，只探测能力是否存在，没有就说明缺什么、给出装法，然后降级继续干活。契约与实测过的安装命令见 `reference/doc-sources.md`。 diff --git a/plugins/pilot/skills/pilot/SKILL.md b/plugins/pilot/skills/pilot/SKILL.md index e5e8117..7110334 100644 --- a/plugins/pilot/skills/pilot/SKILL.md +++ b/plugins/pilot/skills/pilot/SKILL.md @@ -46,7 +46,7 @@ pilot doctor # 自检本地就绪度（config/docs/分支/gh/hook）——** 1. **绝不 `git add -A` / `git add .`**。只 `git-guard.sh add <显式路径>`（裸 `git add -A` 会被 git-guard 拒绝）。理由：`-A` 会把未确认是否该跟踪的文件（密钥、`.env`、构建产物、临时文件）一起提交，是最危险的日常动作。提交前先 `git status` 看清，逐一列出要提交的路径。 2. **绝不直接 push 到主干（main/master），绝不直接合并自己的 PR 到主干**。push 走 `git-guard.sh push`、**开 PR 走 `git-guard.sh pr-create`**(先 `preflight.sh run` 让本仓库的检查真跑过)、合并走 `git-guard.sh merge-pr --integration `（推主干 / 合并 base≠集成分支都会被硬拒绝）。所有代码变更走：feature 分支 → PR → review → 合并到**集成分支**（默认 `preview`，见 `.pilot.yml`）。主干只由集成分支经受控流程进入。**单主干仓库**（没有集成分支，PR 直接开向 `main`）加 `--allow-trunk`——它**不是绕过**：仍要求该分支的 GitHub 保护规则要求审批、且这个 PR 已经 `APPROVED`，读不到保护规则就拒绝（fail-closed）。 3. **一个 task = 一个分支 = 一个（可选）worktree = 一个 PR**。不在一个分支里顺手做别的 task。 -4. **删除分支只用 `git branch -d`，永不 `-D`**；删除只针对「已合并 + 干净」的分支/worktree；一切经 `scripts/safe-cleanup.sh`，默认 dry-run。 +4. **删除分支默认只用 `git branch -d`**；删除只针对「已合并 + 干净」的分支/worktree；一切经 `scripts/safe-cleanup.sh`，默认 dry-run。**唯一的 `-D` 例外**：squash-merge 仓库里 `git branch --merged` 恒为空，此时 `--squash-merged` 会按 commit 向 GitHub 核实「哪个已合并 PR 引入了它」，**有服务端证据才删**（详见 `reference/git-safety.md`）。 5. **PR 之前必须自审 + 对抗 review**（怎么审见 `reference/pr-quality.md`，**审几轮由 `scripts/grade-change.sh` 机械定级，见 `reference/pre-pr-review.md`**——A/B 级 3 轮，不是作者自己说了算）。没过 review 的代码不进 PR，没 approve 的 PR 不合并。 6. **状态即文档**。每推进一步都更新 `docs/agent/tasks.md` 与 `docs/agent/progress.md`；宁可慢，不可让文档与仓库真实状态脱节。 7. **无人值守时不猜产品决策**。遇到影响产品方向/验收/架构的未知，把相关 task 标 `BLOCKED` 并记录待决问题，继续做不受影响的 task；绝不擅自替用户拍板。 diff --git a/plugins/pilot/skills/pilot/reference/git-safety.md b/plugins/pilot/skills/pilot/reference/git-safety.md index 438fff8..c8924f1 100644 --- a/plugins/pilot/skills/pilot/reference/git-safety.md +++ b/plugins/pilot/skills/pilot/reference/git-safety.md @@ -25,7 +25,17 @@ - 一个 task = 一个分支 = 一个（可选）worktree = 一个 PR。分支命名 `/-`，如 `feat/T1.3.2-admin-init`。 ## 删除（清理） -- **删本地分支只用 `git branch -d`，永不 `-D`**。`-d` 会拒绝删除未合并分支，是安全网；`-D` 强删会丢未合并工作。 +- **删本地分支默认只用 `git branch -d`**。`-d` 会拒绝删除未合并分支，是安全网；`-D` 强删会丢未合并工作。 +- **唯一的 `-D` 例外：squash-merge 仓库。** 那里 `git branch --merged` **恒返回 0** —— squash 重写补丁， + 原 commit 不是集成分支的祖先。实测本仓库 28 个分支、`git branch --merged main` 返回 0， + 于是这个脚本在自己家里**什么都清理不了**，人只能手工 `-D`，比脚本存在还糟。 + 所以 `safe-cleanup.sh --squash-merged` 引入第二种**服务端**证据：GitHub 的 + `/commits/{sha}/pulls` 告诉你「哪个 PR 把这个 commit 引入了仓库」，只有当它给出 + `merged_at != null` 的 PR 时才允许 `-D`。 + **按 commit 判、不按分支名判**，因为两个方向的错都真实发生过： + ① 漏删 —— `work-pr18` 这类分支名从没当过 PR head，但 tip 就是别的 PR 的已合并 head； + ② 误删 —— 分支名可复用，同名分支删掉重开后内容全不同，旧的 MERGED PR 仍然匹配名字。 + 没证据 / 没装 gh / 没登录 → **一律保留**，且明确报「无法核实」而不是「没有可清理的」。 - 只删「已合并进集成分支 + 干净」的分支/worktree。 - 一切经 `scripts/safe-cleanup.sh`，默认 dry-run，`--apply` 才执行。**不要在对话里手工逐个删**，避免漏判保护分支。 - 删远程分支（`--remote`）需 `.pilot.yml` 里 `allow_remote_cleanup: true` + 用户明确同意；无人值守默认不删远程。 diff --git a/plugins/pilot/skills/pilot/scripts/git-guard.sh b/plugins/pilot/skills/pilot/scripts/git-guard.sh index 7a7a5ad..76e183a 100755 --- a/plugins/pilot/skills/pilot/scripts/git-guard.sh +++ b/plugins/pilot/skills/pilot/scripts/git-guard.sh @@ -8,7 +8,7 @@ # Usage: # git-guard.sh add [...] # git-guard.sh push -# git-guard.sh merge-pr --integration [--allow-trunk] [extra gh args...] +# git-guard.sh merge-pr --integration [--allow-trunk] [allowlisted gh flags] # # Exit codes: 2 = usage error, 3 = BLOCKED by a rail. On success it execs the real command. set -euo pipefail @@ -167,16 +167,31 @@ case "$sub" in # routed around with a bare `gh pr merge`, which teaches people to route around guards. # So: opt in explicitly, and the opt-in still has to PROVE the danger is handled (below). --allow-trunk) allow_trunk=1; shift ;; - # Refuse flags that defeat the rail: --admin bypasses branch protection; --repo/-R - # would point the merge at a different repo than the base check validated. - # Prefix/attached-value forms bypass an exact-string blocklist: --admin=true, --repo=o/r, - # -Ro/r all defeat the rail, so match those shapes too. - --admin|--admin=*|--repo|--repo=*|-R|-R*) die "refusing '$1' on merge-pr — it would bypass the safety rail" ;; - # --delete-branch/-d deletes the PR's HEAD branch after merge; a PR with head=main would - # delete trunk. The head-branch protected-check below is the real guard; also refuse the - # flag outright so branch cleanup stays an explicit, separate safe-cleanup.sh decision. - -d|--delete-branch|--delete-branch=*) die "refusing '$1' on merge-pr — deletes the PR head branch; clean up branches via safe-cleanup.sh" ;; - *) args+=("$1"); shift ;; + # ---- ALLOWLIST, not a denylist ------------------------------------------------------- + # A denylist can only refuse the dangerous flags that exist TODAY. When `gh pr merge` + # grows a new one that defeats branch protection, a denylist silently passes it through + # and nothing here notices. An allowlist fails the other way: an unrecognised flag is + # refused until someone deliberately adds it — which is the direction a guard should + # fail. (The previously-denied `--admin` / `--repo` / `-R` / `--delete-branch` are simply + # absent from the list below; the two that people actually reach for keep their specific + # error messages so the refusal explains itself.) + --squash|--merge|--rebase|--auto) args+=("$1"); shift ;; + # Value-taking, in both the separate and attached forms. + --body|-b|--body-file|-F|--subject|-t|--match-head-commit) + [ $# -ge 2 ] || die "'$1' requires a value" + args+=("$1" "$2"); shift 2 ;; + --body=*|--body-file=*|--subject=*|--match-head-commit=*) args+=("$1"); shift ;; + # Kept as named refusals purely for the message — the allowlist would reject them anyway. + --admin|--admin=*|--repo|--repo=*|-R|-R*) + die "refusing '$1' on merge-pr — it would bypass the safety rail" ;; + -d|--delete-branch|--delete-branch=*) + die "refusing '$1' on merge-pr — deletes the PR head branch; clean up branches via safe-cleanup.sh" ;; + -*) + die "refusing unrecognised flag '$1' on merge-pr — this is an ALLOWLIST. + Permitted: --squash --merge --rebase --auto --body/-b --body-file/-F --subject/-t --match-head-commit + If '$1' is genuinely safe, add it to the allowlist in git-guard.sh with a note on why." ;; + *) + die "refusing extra positional argument '$1' on merge-pr — the PR number is the first argument and there are no others" ;; esac done [ -n "$n" ] || die "usage: git-guard.sh merge-pr --integration [--allow-trunk] [gh args]" diff --git a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh index b5f91a8..bc0fa5a 100755 --- a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh +++ b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh @@ -3,23 +3,49 @@ # # Guarantees (do not weaken these): # * Dry-run by default. Nothing is deleted unless --apply is passed. -# * Local branches: only `git branch -d` (git refuses to delete unmerged). NEVER `-D`. +# * Local branches: `git branch -d` only (git itself refuses to delete unmerged work). +# The single exception is a branch with SERVER-SIDE proof it was squash-merged — see +# "Squash-merged branches" below. It needs --squash-merged, --apply, and the proof. # * Never touch: the current branch, the integration branch, or any protected branch. # * Worktrees: removed only if their working tree is CLEAN and their branch is merged. # * Remote branches: only deleted with --remote (opt-in) AND only if merged + not protected. # * A dirty worktree — and its remote branch — is always left completely alone. # -# Known limitation (intentional, conservative): squash-merged branches are not -# detected by `git branch --merged`, so they are SKIPPED, not deleted. Missing a -# cleanup is safe; a wrong deletion is not. Delete those by hand if you want them gone. +# Squash-merged branches (--squash-merged, opt-in): +# In a squash-merge repo `git branch --merged` returns NOTHING, ever — the squash rewrites the +# patch so the original commits are not ancestors of the integration branch. Measured here: +# 28 local branches, `git branch --merged main` → 0. That made this script unable to clean +# anything at all in its own home repo, and an unusable guard gets replaced by hand-run `-D`, +# which is strictly worse than the guard existing. +# +# So `--squash-merged` adds a SECOND source of merge evidence, and it is a real one, not a +# loosening: GitHub's `/commits/{sha}/pulls` says which PR introduced a commit to the +# repository. A branch qualifies only when that endpoint names a PR with `merged_at != null` +# for the branch's tip commit. +# +# Why keyed on the COMMIT and not the branch name — both directions of the name-based mistake +# are real and were hit here: +# * miss — `work-pr18` / `worktree-agent-*` were never any PR's head branch, yet their tips +# were the merged heads of #18 / #23. A name lookup calls them unmerged. +# * WRONG DELETE — branch names are reusable. Delete a branch, recreate it with unrelated +# work, and the old MERGED PR still matches the name. A name lookup deletes it. +# The commit-keyed check has neither failure mode. Verified against all four shapes: +# squash-merged head ✓, mid-branch commit ✓, closed-but-unmerged → no evidence ✓, +# never-had-a-PR → no evidence ✓. +# +# Deleting these needs `git branch -D` (`-d` refuses, correctly — git cannot see the merge). +# That is the ONLY place -D is ever used, it requires --squash-merged AND --apply, and it +# requires the evidence above. No evidence, no gh, no auth → the branch is KEPT. # # Usage: -# safe-cleanup.sh [--integration ] [--protect "a,b,c"] [--apply] [--remote] [--remote-name origin] +# safe-cleanup.sh [--integration ] [--protect "a,b,c"] [--apply] [--squash-merged] +# [--remote] [--remote-name origin] set -euo pipefail integration="" protect_csv="main,master,develop,preview,integration,release,hotfix" apply=0 +squash_merged=0 do_remote=0 remote_name="origin" @@ -28,6 +54,7 @@ while [ $# -gt 0 ]; do --integration) integration="${2:-}"; shift 2 ;; --protect) protect_csv="${protect_csv},${2:-}"; shift 2 ;; --apply) apply=1; shift ;; + --squash-merged) squash_merged=1; shift ;; --remote) do_remote=1; shift ;; --remote-name) remote_name="${2:-origin}"; shift 2 ;; *) shift ;; @@ -68,6 +95,37 @@ is_protected() { return 1 } +# ---- squash-merge evidence (read-only; no fetch, no ref writes, so dry-run stays read-only) ---- +# Resolved once, lazily: most repos never need it, and `gh` may not be installed at all. +gh_state="unknown" # unknown | ready | unavailable +gh_repo="" +gh_init() { + [ "$gh_state" = "unknown" ] || return 0 + gh_state="unavailable" + command -v gh >/dev/null 2>&1 || return 0 + gh auth status >/dev/null 2>&1 || return 0 + gh_repo="$(gh repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null || true)" + [ -n "$gh_repo" ] && gh_state="ready" + return 0 +} + +# Echo the number of a MERGED PR that introduced this branch's tip commit, or nothing. +# Nothing = no evidence = keep the branch. Every failure path (no gh, no auth, API error, +# unparseable answer) lands on "nothing" — the check can only ever ADD permission to delete. +merged_pr_for() { + local b="$1" sha out + gh_init + [ "$gh_state" = "ready" ] || return 0 + sha="$(git rev-parse --verify --quiet "$b^{commit}" 2>/dev/null)" || return 0 + [ -n "$sha" ] || return 0 + out="$(gh api "repos/$gh_repo/commits/$sha/pulls" \ + --jq '[.[] | select(.merged_at != null) | .number] | first // empty' 2>/dev/null || true)" + case "$out" in + ''|*[!0-9]*) return 0 ;; # empty, error text, or anything non-numeric → no evidence + *) printf '%s' "$out" ;; + esac +} + mode="DRY-RUN (pass --apply to execute)" [ "$apply" = "1" ] && mode="APPLY" echo "== pilot safe-cleanup ==" @@ -108,6 +166,53 @@ else fi echo +# ---- 1b. Squash-merged local branches (evidence-based; opt-in) --------------- +# Separate section on purpose: these need `-D`, so they must never be confused with the +# `-d`-safe list above. A branch appears here ONLY with a merged-PR number attached. +echo "## Squash-merged local branches" +# Probe HERE, in the main shell. `merged_pr_for` is only ever called as `$( ... )`, which runs in +# a SUBSHELL — anything it assigns to gh_state is discarded on return. Relying on that left the +# "cannot verify" branch permanently unreachable, so a missing/unauthenticated gh printed the +# same "(none)" as a genuinely clean repo. "Could not check" must never look like "nothing found". +gh_init +sq_names=() +sq_prs=() +while IFS= read -r b; do + b="$(echo "$b" | sed 's/^[* +] *//')" + [ -z "$b" ] && continue + is_protected "$b" && continue + is_in_worktree "$b" && continue + # Anything git already calls merged was handled above. + git branch --merged "$integration" 2>/dev/null | sed 's/^[* +] *//' | grep -Fqx "$b" && continue + pr="$(merged_pr_for "$b")" + [ -z "$pr" ] && continue + sq_names+=("$b"); sq_prs+=("$pr") +done < <(git branch --format='%(refname:short)' 2>/dev/null) + +if [ "${#sq_names[@]}" -eq 0 ]; then + if [ "$gh_state" = "unavailable" ]; then + echo " (cannot verify — gh not installed/authenticated, or repo not resolvable; branches KEPT)" + else + echo " (none)" + fi +else + i=0 + while [ "$i" -lt "${#sq_names[@]}" ]; do + b="${sq_names[$i]}"; pr="${sq_prs[$i]}" + if [ "$squash_merged" = "1" ] && [ "$apply" = "1" ]; then + # -D, justified by the merged-PR evidence just gathered for THIS tip commit. + if git branch -D -- "$b" >/dev/null 2>&1; then echo " deleted $b (merged via PR #$pr)" + else echo " SKIP (delete failed) $b"; fi + elif [ "$squash_merged" = "1" ]; then + echo " would delete $b (merged via PR #$pr)" + else + echo " candidate $b (merged via PR #$pr) — pass --squash-merged to include" + fi + i=$((i + 1)) + done +fi +echo + # ---- 2. Worktrees: clean + merged only -------------------------------------- echo "## Worktrees (clean + merged only)" main_root="$(git rev-parse --show-toplevel)" From 8032203c77a7aad8b0af67b5c848d1f8ae7ce238 Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Wed, 5 Aug 2026 14:22:01 +0700 Subject: [PATCH 02/12] =?UTF-8?q?docs:=20=E8=B4=A6=E6=9C=AC=20FU-1..FU-4?= =?UTF-8?q?=20=E5=85=A8=E9=83=A8=E6=A0=87=E8=AE=B0=20done=3DPR#45?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 四条都在 #45 里落地:FU-1 白名单、FU-2+FU-4 safe-cleanup 支持 squash 仓库、 FU-3 已在 #43 顺手做掉(check-version-sync 的 SCOPE 注释)。待做归零。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- docs/agent/followups.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/agent/followups.md b/docs/agent/followups.md index 33e907d..944974a 100644 --- a/docs/agent/followups.md +++ b/docs/agent/followups.md @@ -22,7 +22,7 @@ > squash 后的 head ✓、分支中间的 commit ✓、CLOSED 未合并 → 无证据 ✓、从未开过 PR → 无证据 ✓。 > 落地在 `safe-cleanup.sh` 的 `merged_pr_for()`。 -- [ ] FU-1 · B · src=PR#39 review [Low] · 2026-08-05 · git-guard merge-pr 拒绝 gh flag 用的是黑名单(--admin/--repo/-R 及其粘连形式)。黑名单追不上新 flag —— 以后 gh pr merge 若新增能绕过分支保护的 flag,这里不会自动知道。改成白名单(只放行 --squash/--merge/--rebase 等已知安全 flag)更耐久 -- [ ] FU-2 · B · src=2026-08-05 合并 #38/#39/#40/#41 后实测 · 2026-08-05 · safe-cleanup.sh 在 squash-merge 仓库里永远清不掉任何分支:本仓库 28 个本地分支,git branch --merged main 返回 0 个,因为 squash 后原 commit 不是 main 的祖先,而 safe-cleanup 只用 -d 永不 -D。这是继 #39(死代码)、#40(随机红灯)之后同一家族的第三个『守卫跑不起来』。正确改法:用 gh 核实『存在 headRefName==该分支且 state==MERGED 的 PR』作为已合并证据,再允许 -D;不能简单放开 -D -- [ ] FU-3 · C · src=PR#42 review [Low] · 2026-08-05 · check-version-sync.sh 只比对 plugin.json 与 SKILL.md 两处。今天 README 没有硬编码版本号(核过),所以没问题;但哪天 README 加上版本,这条守卫不会知道。在脚本里写一句把范围钉住:『目前只有这两处声明版本』 -- [ ] FU-4 · B · src=PR#42 review [Low] + 2026-08-05 清理 28 个分支的实测 · 2026-08-05 · 补充 FU-2 的实现要点(今天手工做过一遍,算法已验证):① git branch --merged 和 git cherry 在 squash 仓库里【全部失效】—— cherry 对 12 个分支全报『未在 main』,因为 squash 重写补丁、patch-id 永不匹配;② 可用判据是『本地 tip == 或 是 任何一个已合并 PR 的 headRefOid 的祖先』,要先 git fetch origin pull/N/head 把 head 抓到本地;③ 【不能只按分支名匹配 PR】—— work-pr18 / fix-pr18-round2 / worktree-agent-* 这三个分支名从没当过 PR head,但 tip 就是 PR#18/#23 的已合并 head,按名字匹配会漏掉;④ 反向风险(评审提的):分支名可复用,同名分支删掉重开后内容不同,旧 MERGED PR 仍在 —— 祖先检查恰好挡住这种情况(重开的 tip 不会是旧 head 的祖先),但若改成只按名字匹配就会误删 +- [x] FU-1 · B · src=PR#39 review [Low] · 2026-08-05 · git-guard merge-pr 拒绝 gh flag 用的是黑名单(--admin/--repo/-R 及其粘连形式)。黑名单追不上新 flag —— 以后 gh pr merge 若新增能绕过分支保护的 flag,这里不会自动知道。改成白名单(只放行 --squash/--merge/--rebase 等已知安全 flag)更耐久 · done=PR#45 +- [x] FU-2 · B · src=2026-08-05 合并 #38/#39/#40/#41 后实测 · 2026-08-05 · safe-cleanup.sh 在 squash-merge 仓库里永远清不掉任何分支:本仓库 28 个本地分支,git branch --merged main 返回 0 个,因为 squash 后原 commit 不是 main 的祖先,而 safe-cleanup 只用 -d 永不 -D。这是继 #39(死代码)、#40(随机红灯)之后同一家族的第三个『守卫跑不起来』。正确改法:用 gh 核实『存在 headRefName==该分支且 state==MERGED 的 PR』作为已合并证据,再允许 -D;不能简单放开 -D · done=PR#45 +- [x] FU-3 · C · src=PR#42 review [Low] · 2026-08-05 · check-version-sync.sh 只比对 plugin.json 与 SKILL.md 两处。今天 README 没有硬编码版本号(核过),所以没问题;但哪天 README 加上版本,这条守卫不会知道。在脚本里写一句把范围钉住:『目前只有这两处声明版本』 · done=PR#45 +- [x] FU-4 · B · src=PR#42 review [Low] + 2026-08-05 清理 28 个分支的实测 · 2026-08-05 · 补充 FU-2 的实现要点(今天手工做过一遍,算法已验证):① git branch --merged 和 git cherry 在 squash 仓库里【全部失效】—— cherry 对 12 个分支全报『未在 main』,因为 squash 重写补丁、patch-id 永不匹配;② 可用判据是『本地 tip == 或 是 任何一个已合并 PR 的 headRefOid 的祖先』,要先 git fetch origin pull/N/head 把 head 抓到本地;③ 【不能只按分支名匹配 PR】—— work-pr18 / fix-pr18-round2 / worktree-agent-* 这三个分支名从没当过 PR head,但 tip 就是 PR#18/#23 的已合并 head,按名字匹配会漏掉;④ 反向风险(评审提的):分支名可复用,同名分支删掉重开后内容不同,旧 MERGED PR 仍在 —— 祖先检查恰好挡住这种情况(重开的 tip 不会是旧 head 的祖先),但若改成只按名字匹配就会误删 · done=PR#45 From 6fc9502ac9e66f25e9acbdbbd6442177be0ba4b1 Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Wed, 5 Aug 2026 14:42:09 +0700 Subject: [PATCH 03/12] =?UTF-8?q?fix(pilot):=20status=20=E9=98=B6=E6=AE=B5?= =?UTF-8?q?=E6=8E=A5=E4=B8=8A=20--squash-merged=20=E2=80=94=E2=80=94=20?= =?UTF-8?q?=E5=90=A6=E5=88=99=E4=BF=AE=E7=9A=84=E4=B8=9C=E8=A5=BF=E5=88=B0?= =?UTF-8?q?=E4=B8=8D=E4=BA=86=E8=B0=83=E7=94=A8=E6=96=B9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 自查发现的:#45 给 safe-cleanup 加了 --squash-merged,但 phases/status.md —— **唯一调用它的地方** —— 完全没提这个 flag,纪律段还写着「绝不 -D」。 结果是 squash 仓库里 `pilot status` 照样报「没有可清理的」,新能力等于没接上。 这就是本仓库反复栽的那个形状:规范文本指向脚本没有的行为,或者反过来 —— 脚本有了能力而文档不知道。修 FU-2 只改脚本不改调用方,等于没修。 三处: - 清理计划:说明 squash 仓库里「Local merged branches」恒为 (none), 候选在「Squash-merged local branches」一节,并给出带 --squash-merged 的命令 - --apply 那条命令补上 [--squash-merged] - 纪律段:「绝不 -D」改成「-D 只有一个出口:--squash-merged,且必须拿到服务端证据」 另外明确写了:该节若打印 (cannot verify …),那是**查不了**不是**没有**, 要照实说,不要报成「没有可清理的」—— 这条正是 #45 自审时撞出来的坑。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- plugins/pilot/skills/pilot/phases/status.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/plugins/pilot/skills/pilot/phases/status.md b/plugins/pilot/skills/pilot/phases/status.md index afde9a7..f101095 100644 --- a/plugins/pilot/skills/pilot/phases/status.md +++ b/plugins/pilot/skills/pilot/phases/status.md @@ -22,10 +22,19 @@ bash /scripts/safe-cleanup.sh --integration [--protect ""] ``` （**dry-run**，只打印 would-delete / would-remove / KEEP）。把结果原样呈现。 + - **本仓库用 squash 合并时必看**：`git branch --merged` 在 squash 仓库里**恒返回 0**， + 上面那条命令的「Local merged branches」会永远是 `(none)`。脚本会在 + 「Squash-merged local branches」一节把候选列出来（每条附已合并的 PR 号）， + 此时改用： + ``` + bash /scripts/safe-cleanup.sh --integration --squash-merged + ``` + 若该节打印 `(cannot verify …)`，说明 `gh` 没装/没登录 —— 那是**查不了**，不是**没有**， + 照实说，不要报成「没有可清理的」。 5. **征询清理**：把 dry-run 计划给用户，**问是否执行**。得到确认后才加 `--apply`： ``` - bash /scripts/safe-cleanup.sh --integration --apply + bash /scripts/safe-cleanup.sh --integration [--squash-merged] --apply ``` - 要连带删远程已合并分支：仅当 `.pilot.yml` 的 `allow_remote_cleanup: true`，且用户明确同意，才加 `--remote`。 - 无人值守模式（由 /loop 调用且用户已预先授权清理）：可直接 `--apply`，但**永远不加 `--remote`** 除非配置显式开启。 @@ -34,6 +43,6 @@ ## 纪律 -- 清理的所有安全判断在 `safe-cleanup.sh` 里确定性执行（只删已合并+干净、只 `-d`、护住 main/集成/当前/protected/脏 worktree）。**不要在对话里手工 `git branch -d`** —— 走脚本，避免漏判。 -- 绝不 `-D`，绝不删未合并分支，绝不碰脏 worktree 或其远程分支。 +- 清理的所有安全判断在 `safe-cleanup.sh` 里确定性执行（只删已合并+干净、护住 main/集成/当前/protected/脏 worktree）。**不要在对话里手工 `git branch -d`/`-D`** —— 走脚本，避免漏判。 +- **`-D` 只有一个出口**：`--squash-merged`，且必须拿到服务端证据（GitHub 说某个已合并 PR 引入了该分支 tip 的那个 commit）。没证据、没 `gh`、没登录 → 保留。除此之外绝不 `-D`，绝不删未合并分支，绝不碰脏 worktree 或其远程分支。 - 汇报前若对某个数字存疑，重跑 `repo-scan.sh` 核对，不要凭记忆报数。 From 7f9dd08fb1fb0f523b84667fc407649905b3964a Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Wed, 5 Aug 2026 15:55:23 +0700 Subject: [PATCH 04/12] =?UTF-8?q?fix(pilot):=20=E4=BF=AE=E8=AF=84=E5=AE=A1?= =?UTF-8?q?=E5=9B=9B=E6=9D=A1=20=E2=80=94=E2=80=94=20=E8=AF=81=E6=8D=AE?= =?UTF-8?q?=E6=94=B6=E6=95=9B=E5=88=B0=20integration=20/=20=E6=8B=92?= =?UTF-8?q?=E6=9C=AA=E7=9F=A5=E5=8F=82=E6=95=B0=20/=20=E5=8C=BA=E5=88=86?= =?UTF-8?q?=E6=9F=A5=E4=B8=8D=E4=BA=86=20/=20worktree?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 四条全部成立,不辩。逐条: ## B1 证据从不按 --integration 收敛(采纳一半,另一半会把特性变回死代码) **采纳**:jq 加 `.base.ref == "$integration"`。原判据是「仓库里*某个*已合并 PR 关联了这个 tip」, stacked PR 就能满足:子分支合进父 feature 分支、父分支的 PR 后来 closed 未合并 —— 子分支 tip 从此永远带着「已合并」证据,工作却从没进集成分支。 评审在沙箱里复现了,我复测确认:同一个分支,`--integration main` 给出证据、 `--integration cla-signatures` 证据消失。 **不采纳**:额外要求 `git merge-base --is-ancestor $sha $integration`。 squash 仓库里 tip **按构造**永远不是集成分支的祖先 —— 这正是本函数存在的理由。 加上它等于把特性退回它要修的那个死代码状态。base 收敛是正确的收窄,祖先性在这里不可用。 理由写进函数注释,免得下次有人再加。 ## B2 参数解析静默吞未知参数 `--integraton main`(少一个字母)整条被丢弃,integration 悄悄回退到 origin/HEAD 猜的分支。 评审说得对而且刺眼:**同一个 commit 里的 git-guard 刚为完全相同的理由从黑名单改成白名单**。 现在 unknown → exit 2 并打 usage;带值参数缺值也报错。 ## B3 「查不了」在逐分支这一层又变回「没有」 gh_state 只记 init 级失败;单分支 gh api 的 403/5xx 落在 `|| true` 里,和「无证据」同形。 而限流恰恰是最可能的失败 —— 这个特性每分支一次 API。 现在 merged_pr_for 返回三态:PR 号 / ERR / 空。有失败就打 「N branch(es) could not be verified — KEPT」,**有候选时也追加一行说明列表不完整**, 绝不在有失败时打印裸 (none)。 实测:stub 一个 auth/repo 成功但 api 恒 403 的 gh → 输出 「(4 branch(es) could not be verified — KEPT. NOT the same as 'nothing to clean'.)」 ## R4 worktree —— 第四例,而且是在修第三例的同一个 PR 里留下的 第 2 节的合并判据仍只有 `git branch --merged`,也就是本 PR 花 30 行论证「squash 仓库里恒返回 0」 的那个谓词。结果:squash 仓库里任何 worktree 永远清不掉,哪怕证据齐全。 而 SKILL.md 的教条是「一个 task = 一个分支 = 一个 **worktree** = 一个 PR」,worktree 才是主导单位。 现在 worktree 走同一套证据:git 原生 → `-d`;squash 证据 → `-D`;ERR → KEEP 并说「查不了」; 有证据但没给 flag → KEEP 并提示加 --squash-merged。 ## 实测 - B1:同分支换 --integration,证据随之出现/消失 - B2:拼错参数 → unknown argument + usage;缺值 → requires a value - B3:gh api 恒 403 → 「4 branch(es) could not be verified — KEPT」,不是 (none) - R4:带证据的干净 worktree → 无 flag 时提示、有 flag 时 would remove; **脏 worktree 仍然 KEEP(dirty, 1 changes)** —— 安全性没被这次改动破坏 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- .../skills/pilot/scripts/safe-cleanup.sh | 107 +++++++++++++++--- 1 file changed, 89 insertions(+), 18 deletions(-) diff --git a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh index bc0fa5a..499afd1 100755 --- a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh +++ b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh @@ -49,15 +49,25 @@ squash_merged=0 do_remote=0 remote_name="origin" +# Unknown arguments are REFUSED, not skipped. `*) shift ;;` silently dropped a typo like +# `--integraton main`, and `integration` then fell back to a guess from origin/HEAD — so +# `--apply` would delete against a baseline the caller never asked for. `--integration` is the +# one argument whose typo is unrecoverable, and run.md requires callers to pass it explicitly. +# (Same reasoning that turned git-guard's flag denylist into a refuse-unknown allowlist in this +# very commit; it applies here for identical reasons.) +need_val() { [ "$2" -ge 2 ] || { echo "safe-cleanup: '$1' requires a value" >&2; exit 2; }; } while [ $# -gt 0 ]; do case "$1" in - --integration) integration="${2:-}"; shift 2 ;; - --protect) protect_csv="${protect_csv},${2:-}"; shift 2 ;; - --apply) apply=1; shift ;; + --integration) need_val "$1" $#; integration="$2"; shift 2 ;; + --protect) need_val "$1" $#; protect_csv="${protect_csv},$2"; shift 2 ;; + --remote-name) need_val "$1" $#; remote_name="$2"; shift 2 ;; + --apply) apply=1; shift ;; --squash-merged) squash_merged=1; shift ;; - --remote) do_remote=1; shift ;; - --remote-name) remote_name="${2:-origin}"; shift 2 ;; - *) shift ;; + --remote) do_remote=1; shift ;; + -h|--help) sed -n '2,40p' "$0"; exit 0 ;; + *) echo "safe-cleanup: unknown argument '$1'" >&2 + echo " usage: safe-cleanup.sh [--integration ] [--protect \"a,b\"] [--apply] [--squash-merged] [--remote] [--remote-name ]" >&2 + exit 2 ;; esac done @@ -109,19 +119,39 @@ gh_init() { return 0 } -# Echo the number of a MERGED PR that introduced this branch's tip commit, or nothing. -# Nothing = no evidence = keep the branch. Every failure path (no gh, no auth, API error, -# unparseable answer) lands on "nothing" — the check can only ever ADD permission to delete. +# Echo one of three things, and the difference matters: +# a MERGED PR whose BASE is $integration introduced this branch's tip commit +# ERR the lookup itself failed (rate limit, 5xx, missing scope) — verdict UNKNOWN +# (empty) the lookup succeeded and found no such PR — no evidence +# +# `.base.ref == $integration` is load-bearing, not decoration. Without it the test is merely +# "some merged PR touched this tip", which a STACKED PR satisfies: child merged into a parent +# feature branch, parent's own PR later closed unmerged — the child's tip then carries merge +# evidence forever while its work never reached the integration branch. Measured in a sandbox: +# a commit `git merge-base --is-ancestor … main` calls unreachable was still offered for +# deletion. Brood happens to have integration == default == main, which masks this entirely; +# pilot's documented default shape is `preview != main`, where it bites. +# +# NB: an `--is-ancestor $sha $integration` test is deliberately NOT added on top. In a +# squash-merge repo the tip is BY CONSTRUCTION never an ancestor of the integration branch — +# that is the whole reason this function exists. Requiring it would return the feature to the +# dead-code state it was written to fix. Base convergence is the correct narrowing; ancestry +# is not available here. merged_pr_for() { - local b="$1" sha out + local b="$1" sha out rc gh_init - [ "$gh_state" = "ready" ] || return 0 + [ "$gh_state" = "ready" ] || { printf 'ERR'; return 0; } sha="$(git rev-parse --verify --quiet "$b^{commit}" 2>/dev/null)" || return 0 [ -n "$sha" ] || return 0 out="$(gh api "repos/$gh_repo/commits/$sha/pulls" \ - --jq '[.[] | select(.merged_at != null) | .number] | first // empty' 2>/dev/null || true)" + --jq "[.[] | select(.merged_at != null and .base.ref == \"$integration\") | .number] | first // empty" 2>/dev/null)" + rc=$? + # A failed CALL is not the same as an empty ANSWER. Rate limiting is the most likely failure + # here precisely because this feature spends one API call per branch. + [ "$rc" -ne 0 ] && { printf 'ERR'; return 0; } case "$out" in - ''|*[!0-9]*) return 0 ;; # empty, error text, or anything non-numeric → no evidence + '') return 0 ;; # 200 with no matching PR → genuinely no evidence + *[!0-9]*) printf 'ERR' ;; # 200 but unparseable → treat as unknown, never as "no" *) printf '%s' "$out" ;; esac } @@ -177,6 +207,7 @@ echo "## Squash-merged local branches" gh_init sq_names=() sq_prs=() +sq_errors=0 while IFS= read -r b; do b="$(echo "$b" | sed 's/^[* +] *//')" [ -z "$b" ] && continue @@ -185,6 +216,13 @@ while IFS= read -r b; do # Anything git already calls merged was handled above. git branch --merged "$integration" 2>/dev/null | sed 's/^[* +] *//' | grep -Fqx "$b" && continue pr="$(merged_pr_for "$b")" + # Count PER-BRANCH lookup failures separately. gh_state only records init-level failure; + # a 403/5xx on an individual branch used to land in the same "no evidence" bucket, so a + # rate-limited run printed a byte-identical "(none)" to a genuinely clean repo — and a + # partially failed run silently dropped the branches it could not check. status.md (added in + # this same PR) tells the model to say "could not verify, not nothing"; it needs this signal + # to be able to. + if [ "$pr" = "ERR" ]; then sq_errors=$((sq_errors + 1)); continue; fi [ -z "$pr" ] && continue sq_names+=("$b"); sq_prs+=("$pr") done < <(git branch --format='%(refname:short)' 2>/dev/null) @@ -192,6 +230,8 @@ done < <(git branch --format='%(refname:short)' 2>/dev/null) if [ "${#sq_names[@]}" -eq 0 ]; then if [ "$gh_state" = "unavailable" ]; then echo " (cannot verify — gh not installed/authenticated, or repo not resolvable; branches KEPT)" + elif [ "$sq_errors" -gt 0 ]; then + echo " ($sq_errors branch(es) could not be verified — KEPT. NOT the same as 'nothing to clean'.)" else echo " (none)" fi @@ -210,6 +250,8 @@ else fi i=$((i + 1)) done + # A partial result must never read as a complete one. + [ "$sq_errors" -gt 0 ] && echo " ($sq_errors more branch(es) could not be verified — KEPT; this list is INCOMPLETE)" fi echo @@ -238,20 +280,49 @@ handle_wt() { echo " KEEP (dirty, $dirty changes) $path [$short]" return fi - # merged check - if [ "$short" = "(detached)" ] || is_protected "$short" || ! git branch --merged "$integration" 2>/dev/null | sed 's/^[* +] *//' | grep -Fqx "$short"; then + # merged check — git-native first, then the same squash evidence section 1b uses. + # Using ONLY `git branch --merged` here would have left worktrees permanently uncleanable in a + # squash repo: that predicate returns 0 rows by construction, which is the entire premise of + # this PR. Missing it meant the fix stopped at loose branches while `SKILL.md`'s own doctrine + # ("一个 task = 一个分支 = 一个 worktree = 一个 PR") makes the worktree the dominant unit, and + # `phases/status.md` reports worktree cleanup on every `pilot status`. + local wt_pr="" via="" + if [ "$short" = "(detached)" ] || is_protected "$short"; then echo " KEEP (not a merged feature branch) $path [$short]" return fi + if git branch --merged "$integration" 2>/dev/null | sed 's/^[* +] *//' | grep -Fqx "$short"; then + via="git" + else + wt_pr="$(merged_pr_for "$short")" + if [ "$wt_pr" = "ERR" ]; then + echo " KEEP (could not verify — lookup failed, NOT 'unmerged') $path [$short]" + return + fi + if [ -z "$wt_pr" ]; then + echo " KEEP (not a merged feature branch) $path [$short]" + return + fi + if [ "$squash_merged" != "1" ]; then + echo " KEEP (squash-merged via PR #$wt_pr) — pass --squash-merged to include $path [$short]" + return + fi + via="squash-PR#$wt_pr" + fi if [ "$apply" = "1" ]; then if git worktree remove "$path" 2>/dev/null; then - echo " removed $path [$short]" - git branch -d -- "$short" 2>/dev/null && echo " + deleted branch $short" || true + echo " removed $path [$short] ($via)" + # -d for git-native evidence; -D only where a merged PR with base=$integration proves it. + if [ "$via" = "git" ]; then + git branch -d -- "$short" 2>/dev/null && echo " + deleted branch $short" || true + else + git branch -D -- "$short" 2>/dev/null && echo " + deleted branch $short ($via)" || true + fi else echo " SKIP (remove failed) $path [$short]" fi else - echo " would remove $path [$short] (+ delete branch $short)" + echo " would remove $path [$short] ($via) (+ delete branch $short)" fi } while IFS= read -r line; do From 33cccf9f7682227c2a24a302cf82dbe2137eda4a Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Wed, 5 Aug 2026 18:26:05 +0700 Subject: [PATCH 05/12] =?UTF-8?q?fix(pilot):=20jq=20=E8=B0=93=E8=AF=8D?= =?UTF-8?q?=E6=94=B9=E7=94=A8=20$ENV=20=E4=BC=A0=E5=8F=82=20=E2=80=94?= =?UTF-8?q?=E2=80=94=20=E5=88=86=E6=94=AF=E5=90=8D=E4=B8=8D=E8=83=BD?= =?UTF-8?q?=E5=86=8D=E6=94=B9=E5=86=99=E5=88=A4=E6=8D=AE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 第二轮评审唯一 blocking,成立。 $integration 原来是 shell 插值进 jq **程序**,不是当**数据**比较。一个 git 合法的分支名 `x"or(true)or"` 会把谓词改写成 `.base.ref == "x"or(true)or""` —— 恒真,于是 base 是别的 分支的已合并 PR 也被当成证据,终点是不可逆的 `git branch -D`。 评审在一次性沙箱里端到端跑到了真删除。这正是第一轮 B1 要堵的失败类, 从**修复自身内部**又绕回来了 —— 入口从「jq 里少一个条件」变成「那个条件可被输入改写」。 改成 PILOT_INTEG 环境变量 + $ENV.PILOT_INTEG,值永远是数据。 实测:--integration main → `would delete _t-inj (merged via PR #42)`(无回归); --integration 'x"or(true)or"' → `(none)`,谓词没被改写。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- plugins/pilot/skills/pilot/scripts/safe-cleanup.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh index 499afd1..a1eca0a 100755 --- a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh +++ b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh @@ -143,8 +143,14 @@ merged_pr_for() { [ "$gh_state" = "ready" ] || { printf 'ERR'; return 0; } sha="$(git rev-parse --verify --quiet "$b^{commit}" 2>/dev/null)" || return 0 [ -n "$sha" ] || return 0 - out="$(gh api "repos/$gh_repo/commits/$sha/pulls" \ - --jq "[.[] | select(.merged_at != null and .base.ref == \"$integration\") | .number] | first // empty" 2>/dev/null)" + # $integration goes in as DATA via $ENV, never interpolated into the jq program. Building the + # program by string substitution let a git-legal branch name rewrite the predicate: with + # `x"or(true)or"` the comparison becomes `.base.ref == "x"or(true)or""` — constantly true — and + # a merged PR whose base was some other branch then counted as evidence, ending in an + # irreversible `git branch -D`. That is the exact failure class the base check was added to + # close, re-entering through the check itself. + out="$(PILOT_INTEG="$integration" gh api "repos/$gh_repo/commits/$sha/pulls" \ + --jq '[.[] | select(.merged_at != null and .base.ref == $ENV.PILOT_INTEG) | .number] | first // empty' 2>/dev/null)" rc=$? # A failed CALL is not the same as an empty ANSWER. Rate limiting is the most likely failure # here precisely because this feature spends one API call per branch. From bc840cfc66d2a4c151e6a41019d7ac77a36210ff Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Wed, 5 Aug 2026 18:32:19 +0700 Subject: [PATCH 06/12] =?UTF-8?q?fix(pilot):=20=E7=AC=AC=E4=BA=8C=E8=BD=AE?= =?UTF-8?q?=E8=AF=84=E5=AE=A1=20=E2=80=94=E2=80=94=20jq=20=E6=B3=A8?= =?UTF-8?q?=E5=85=A5=20+=20protected=20=E8=AF=AD=E4=B9=89=E5=88=86?= =?UTF-8?q?=E6=AD=A7=20+=20=E6=88=90=E6=9C=AC=E9=97=B8=E9=97=A8=20+=206=20?= =?UTF-8?q?=E6=9D=A1=20Low?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 🔴 两条 blocking **① jq 谓词可被分支名改写**(已在 33cccf9 修,本 commit 是配套验证) $integration 走 $ENV 当数据传,不再插值进 jq 程序。 **② safe-cleanup 与 git-guard 的 protected 语义不一致** `"$p"/*` vs `"$p"[-_/.0-9]*` —— 一个 git-guard **拒绝 push** 的 release-1.2, 在 safe-cleanup 里会被列进候选。两个函数体都不是本 PR 改的,但本 PR 新加的 -D 给这个分歧接上了不可逆后果。把 git-guard 的边界集**逐字抄过来**,并写明为什么是 `[-_/.0-9]` 而不是 `*`(挡住 main 吞掉 mainline)。 实测:release-1.2 在 git-guard 被拒推、在 safe-cleanup 候选里命中 0 次。 ## 成本闸门(上轮遗留,这轮更贵了) handle_wt 在检查 --squash-merged **之前**就调 merged_pr_for,导致普通 dry-run 25 次 gh api / 20.6 秒 —— 而 status.md 第 4 步跑的正是这种。 两处(1b 和 handle_wt)都把 flag 判断提到调用之上。 实测(stub 计数):不带 flag **0 次 API / 0 秒**;带 flag 2 次 / 1 秒。 不带 flag 时不静默 —— 打印「(not checked — one gh API call per branch; pass --squash-merged)」 并说明 squash 仓库里可清理的分支只会出现在这一节,否则沉默又会变成「没有」。 ## 6 条 Low - need_val 只数元数不看值:`--protect --apply` 会把 --apply 当值吃掉,脚本报 mode=DRY-RUN 且 exit 0,调用方以为跑过了。现在拒绝 flag 形状的值。 - `--help` 用 `sed -n '2,40p'`,而真正的用法在 41 行之后 —— 一行用法都不显示。改 2,46p。 - **-D 把唯一的恢复句柄扔了**:git 的 "Deleted branch X (was abc1234)." 被 >/dev/null 吞掉。 现在先取 sha,自己打进报告行:`was=6f3da53 → restore: git branch _t 6f3da53`。 实测真删一次后按该句柄成功还原。两处(1b 与 handle_wt)统一。 - ERR 三态只因为两个调用点恰好是命令替换才活着(bash 在 cmdsub 里挂起 errexit)。 改成 `rc=0; out="$(…)" || rc=$?`,扛得住 inherit_errexit 或未来的非-cmdsub 调用方。 - 去掉对无装饰 `%(refname:short)` 多余的 `sed 's/^[* +] *//'`。 - **git-guard 注释与代码相反**:上面刚写「stdout ONLY, Do NOT fold in stderr」, 代码是 `2>/dev/null`,下一句却说「`2>&1` above keeps the error BODY」。 实测代码是对的(gh api 把 error body 放 stdout),**错的是注释** —— 而它挂在 --allow-trunk 这条安全闸门上,下一个维护者照它改成 2>&1 就会把闸门降级成 「cannot read protection」。删掉那句残留。复验两条分支仍可达。 ## 账本 FU-3 的 done 从 PR#45 改成 **PR#43** —— 那条改动实际落在 #43,记错了归属。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- docs/agent/followups.md | 3 +- .../pilot/skills/pilot/scripts/git-guard.sh | 7 +- .../skills/pilot/scripts/safe-cleanup.sh | 69 ++++++++++++++----- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/docs/agent/followups.md b/docs/agent/followups.md index 3e09b71..819b336 100644 --- a/docs/agent/followups.md +++ b/docs/agent/followups.md @@ -24,7 +24,8 @@ - [x] FU-1 · B · src=PR#39 review [Low] · 2026-08-05 · git-guard merge-pr 拒绝 gh flag 用的是黑名单(--admin/--repo/-R 及其粘连形式)。黑名单追不上新 flag —— 以后 gh pr merge 若新增能绕过分支保护的 flag,这里不会自动知道。改成白名单(只放行 --squash/--merge/--rebase 等已知安全 flag)更耐久 · done=PR#45 - [x] FU-2 · B · src=2026-08-05 合并 #38/#39/#40/#41 后实测 · 2026-08-05 · safe-cleanup.sh 在 squash-merge 仓库里永远清不掉任何分支:本仓库 28 个本地分支,git branch --merged main 返回 0 个,因为 squash 后原 commit 不是 main 的祖先,而 safe-cleanup 只用 -d 永不 -D。这是继 #39(死代码)、#40(随机红灯)之后同一家族的第三个『守卫跑不起来』。正确改法:用 gh 核实『存在 headRefName==该分支且 state==MERGED 的 PR』作为已合并证据,再允许 -D;不能简单放开 -D · done=PR#45 -- [x] FU-3 · C · src=PR#42 review [Low] · 2026-08-05 · check-version-sync.sh 只比对 plugin.json 与 SKILL.md 两处。今天 README 没有硬编码版本号(核过),所以没问题;但哪天 README 加上版本,这条守卫不会知道。在脚本里写一句把范围钉住:『目前只有这两处声明版本』 · done=PR#45 +- [x] FU-3 · C · src=PR#42 review [Low] · 2026-08-05 · check-version-sync.sh 只比对 plugin.json 与 SKILL.md 两处。今天 README 没有硬编码版本号(核过),所以没问题;但哪天 README 加上版本,这条守卫不会知道。在脚本里写一句把范围钉住:『目前只有这两处声明版本』 · done=PR#43 + - [x] FU-4 · B · src=PR#42 review [Low] + 2026-08-05 清理 28 个分支的实测 · 2026-08-05 · 补充 FU-2 的实现要点(今天手工做过一遍,算法已验证):① git branch --merged 和 git cherry 在 squash 仓库里【全部失效】—— cherry 对 12 个分支全报『未在 main』,因为 squash 重写补丁、patch-id 永不匹配;② 可用判据是『本地 tip == 或 是 任何一个已合并 PR 的 headRefOid 的祖先』,要先 git fetch origin pull/N/head 把 head 抓到本地;③ 【不能只按分支名匹配 PR】—— work-pr18 / fix-pr18-round2 / worktree-agent-* 这三个分支名从没当过 PR head,但 tip 就是 PR#18/#23 的已合并 head,按名字匹配会漏掉;④ 反向风险(评审提的):分支名可复用,同名分支删掉重开后内容不同,旧 MERGED PR 仍在 —— 祖先检查恰好挡住这种情况(重开的 tip 不会是旧 head 的祖先),但若改成只按名字匹配就会误删 · done=PR#45 - [ ] FU-5 · B · src=2026-08-05 pilot 端到端测试(doctor+plan) · 2026-08-05 · pilot 的起跑门禁只认 docs_dir 下七个固定文件名,认不出等价(且更完整)的规划源。实测:Brood 的规划在 backlog/(4 个 milestone + 49 个带验收标准的 task + 2 个 ADR),check-docs.sh --strict 报 0/7、run 直接 fail-closed 拒跑;而 plan.md A.3 又明写『已有规划 → 不要重复造』—— 两条同时遵守不可能。本次用 docs/agent/ 做适配层(指向 backlog/ 的视图,不复制内容)绕过去了,但根治要给 check-docs.sh 加可配置规划源(如 .pilot.yml 声明 planning_source: backlog),否则每个用 backlog/issues/Jira 管规划的仓库都会被判未就绪 - [ ] FU-6 · C · src=2026-08-05 pilot 端到端测试(doctor) · 2026-08-05 · doctor 第 4 步在单主干仓库里会误导:无 .pilot.yml 时默认 integration_branch=preview,doctor 发现它不存在就『提示先建』—— 但对单主干仓库正确答案是 integration_branch=main + 合并时用 --allow-trunk,不是去建一个 preview 分支。doctor 不知道这两件事是连着的。改法:检测到 preview 不存在但 default branch 存在时,提示单主干配置法并指向 --allow-trunk diff --git a/plugins/pilot/skills/pilot/scripts/git-guard.sh b/plugins/pilot/skills/pilot/scripts/git-guard.sh index 76e183a..d584960 100755 --- a/plugins/pilot/skills/pilot/scripts/git-guard.sh +++ b/plugins/pilot/skills/pilot/scripts/git-guard.sh @@ -243,9 +243,10 @@ case "$sub" in # captured text and makes the JSON unparseable, which silently degrades every branch below # to "cannot read protection". (Measured: merging stderr broke the working case.) prot="$(gh api "repos/$repo/branches/$integration/protection" 2>/dev/null || true)" - # `2>&1` above keeps the API's error BODY (it carries `message`), so parse the captured text - # rather than re-querying. Only a real protection object yields a number; anything else - # (error JSON, empty, HTML) falls through to the fail-closed branch below. + # `gh api` puts the error BODY (which carries `message`) on STDOUT, so the capture above + # holds it without needing stderr — that is why `.message` parsing and the "Branch not + # protected" branch below are reachable. Only a real protection object yields a number; + # anything else (error JSON, empty, HTML) falls through to the fail-closed branch. approvals="$(printf '%s' "$prot" | python3 -c 'import json,sys try: d = json.load(sys.stdin) diff --git a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh index a1eca0a..162570f 100755 --- a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh +++ b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh @@ -55,16 +55,22 @@ remote_name="origin" # one argument whose typo is unrecoverable, and run.md requires callers to pass it explicitly. # (Same reasoning that turned git-guard's flag denylist into a refuse-unknown allowlist in this # very commit; it applies here for identical reasons.) -need_val() { [ "$2" -ge 2 ] || { echo "safe-cleanup: '$1' requires a value" >&2; exit 2; }; } +# Counting arity is not enough: `--protect --apply` passed the arity test and swallowed --apply, +# so the script printed mode=DRY-RUN and exited 0 while the caller believed it had run. Same +# silent-swallow class B2 exists to close. +need_val() { + [ "$2" -ge 2 ] || { echo "safe-cleanup: '$1' requires a value" >&2; exit 2; } + case "$3" in -*) echo "safe-cleanup: '$1' requires a value, got flag '$3'" >&2; exit 2 ;; esac +} while [ $# -gt 0 ]; do case "$1" in - --integration) need_val "$1" $#; integration="$2"; shift 2 ;; - --protect) need_val "$1" $#; protect_csv="${protect_csv},$2"; shift 2 ;; - --remote-name) need_val "$1" $#; remote_name="$2"; shift 2 ;; + --integration) need_val "$1" $# "${2:-}"; integration="$2"; shift 2 ;; + --protect) need_val "$1" $# "${2:-}"; protect_csv="${protect_csv},$2"; shift 2 ;; + --remote-name) need_val "$1" $# "${2:-}"; remote_name="$2"; shift 2 ;; --apply) apply=1; shift ;; --squash-merged) squash_merged=1; shift ;; --remote) do_remote=1; shift ;; - -h|--help) sed -n '2,40p' "$0"; exit 0 ;; + -h|--help) sed -n '2,46p' "$0"; exit 0 ;; *) echo "safe-cleanup: unknown argument '$1'" >&2 echo " usage: safe-cleanup.sh [--integration ] [--protect \"a,b\"] [--apply] [--squash-merged] [--remote] [--remote-name ]" >&2 exit 2 ;; @@ -100,7 +106,12 @@ is_protected() { p="${p%/}" # tolerate trailing slash (e.g. "hotfix/") [ -z "$p" ] && continue [ "$name" = "$p" ] && return 0 - case "$name" in "$p"/*) return 0 ;; esac # e.g. release/* protects release/1.2 + # Boundary set copied VERBATIM from git-guard.sh's is_protected. The two scripts ship in the + # same skill and must not disagree about what "protected" means: with the old `"$p"/*` a + # `release-1.2` that git-guard REFUSES to push to was still offered here as a deletion + # candidate. Harmless while this script only used `-d`; this PR added `-D`, which turns the + # divergence into data loss. `[-_/.0-9]` (not `*`) so `main` cannot swallow `mainline`. + case "$name" in "$p"[-_/.0-9]*) return 0 ;; esac done return 1 } @@ -149,9 +160,14 @@ merged_pr_for() { # a merged PR whose base was some other branch then counted as evidence, ending in an # irreversible `git branch -D`. That is the exact failure class the base check was added to # close, re-entering through the check itself. + # `rc=0; … || rc=$?` instead of a bare `$?`: under `set -e` a failing command substitution only + # survives because bash suspends errexit inside `$( )` and `inherit_errexit` is off by default. + # Verified: convert either call site to a non-cmdsub form and the script exits 1 silently right + # after printing the section header, truncating the report with no error. This form keeps the + # real status AND survives `shopt -s inherit_errexit` or a future non-cmdsub caller. + rc=0 out="$(PILOT_INTEG="$integration" gh api "repos/$gh_repo/commits/$sha/pulls" \ - --jq '[.[] | select(.merged_at != null and .base.ref == $ENV.PILOT_INTEG) | .number] | first // empty' 2>/dev/null)" - rc=$? + --jq '[.[] | select(.merged_at != null and .base.ref == $ENV.PILOT_INTEG) | .number] | first // empty' 2>/dev/null)" || rc=$? # A failed CALL is not the same as an empty ANSWER. Rate limiting is the most likely failure # here precisely because this feature spends one API call per branch. [ "$rc" -ne 0 ] && { printf 'ERR'; return 0; } @@ -210,12 +226,20 @@ echo "## Squash-merged local branches" # a SUBSHELL — anything it assigns to gh_state is discarded on return. Relying on that left the # "cannot verify" branch permanently unreachable, so a missing/unauthenticated gh printed the # same "(none)" as a genuinely clean repo. "Could not check" must never look like "nothing found". +# COST GATE. Each candidate costs one gh API call; a plain `pilot status` dry-run measured +# 25 calls / 20.6s on this repo, and status.md step 4 runs exactly that. Without --squash-merged +# spend ZERO calls and say so — silence would be indistinguishable from "nothing found", the very +# confusion B3 exists to prevent. +if [ "$squash_merged" != "1" ]; then + echo " (not checked — one gh API call per branch; pass --squash-merged to check)" + echo " NB: in a squash-merge repo the section above is ALWAYS (none), so cleanable" + echo " branches would appear HERE, not there." +else gh_init sq_names=() sq_prs=() sq_errors=0 while IFS= read -r b; do - b="$(echo "$b" | sed 's/^[* +] *//')" [ -z "$b" ] && continue is_protected "$b" && continue is_in_worktree "$b" && continue @@ -245,20 +269,24 @@ else i=0 while [ "$i" -lt "${#sq_names[@]}" ]; do b="${sq_names[$i]}"; pr="${sq_prs[$i]}" - if [ "$squash_merged" = "1" ] && [ "$apply" = "1" ]; then + if [ "$apply" = "1" ]; then # -D, justified by the merged-PR evidence just gathered for THIS tip commit. - if git branch -D -- "$b" >/dev/null 2>&1; then echo " deleted $b (merged via PR #$pr)" + # Capture the sha FIRST and print it ourselves: git's "Deleted branch X (was abc1234)." is + # the ONLY handle for recovering a -D'd branch, and the earlier version sent it to + # /dev/null. Redirect git's stdout to keep the report structured, but surface the sha. + sha_short="$(git rev-parse --short "$b" 2>/dev/null || echo unknown)" + if git branch -D -- "$b" >/dev/null 2>&1; then + echo " deleted $b (merged via PR #$pr) was=$sha_short → restore: git branch $b $sha_short" else echo " SKIP (delete failed) $b"; fi - elif [ "$squash_merged" = "1" ]; then - echo " would delete $b (merged via PR #$pr)" else - echo " candidate $b (merged via PR #$pr) — pass --squash-merged to include" + echo " would delete $b (merged via PR #$pr)" fi i=$((i + 1)) done # A partial result must never read as a complete one. [ "$sq_errors" -gt 0 ] && echo " ($sq_errors more branch(es) could not be verified — KEPT; this list is INCOMPLETE)" fi +fi echo # ---- 2. Worktrees: clean + merged only -------------------------------------- @@ -299,6 +327,11 @@ handle_wt() { fi if git branch --merged "$integration" 2>/dev/null | sed 's/^[* +] *//' | grep -Fqx "$short"; then via="git" + elif [ "$squash_merged" != "1" ]; then + # Same cost gate as section 1b: this ran BEFORE the flag check and was the bulk of the + # 25-call / 20.6s plain dry-run. + echo " KEEP (not merged per git; pass --squash-merged to also check for a squash merge) $path [$short]" + return else wt_pr="$(merged_pr_for "$short")" if [ "$wt_pr" = "ERR" ]; then @@ -309,10 +342,6 @@ handle_wt() { echo " KEEP (not a merged feature branch) $path [$short]" return fi - if [ "$squash_merged" != "1" ]; then - echo " KEEP (squash-merged via PR #$wt_pr) — pass --squash-merged to include $path [$short]" - return - fi via="squash-PR#$wt_pr" fi if [ "$apply" = "1" ]; then @@ -322,7 +351,9 @@ handle_wt() { if [ "$via" = "git" ]; then git branch -d -- "$short" 2>/dev/null && echo " + deleted branch $short" || true else - git branch -D -- "$short" 2>/dev/null && echo " + deleted branch $short ($via)" || true + local wsha; wsha="$(git rev-parse --short "$short" 2>/dev/null || echo unknown)" + git branch -D -- "$short" >/dev/null 2>&1 \ + && echo " + deleted branch $short ($via) was=$wsha → restore: git branch $short $wsha" || true fi else echo " SKIP (remove failed) $path [$short]" From 929d8d51eaf93a7f8467b1e5dabc715dcc50c42a Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Wed, 5 Aug 2026 19:04:03 +0700 Subject: [PATCH 07/12] =?UTF-8?q?fix(pilot):=20=E7=AC=AC=E4=B8=89=E8=BD=AE?= =?UTF-8?q?=20=E2=80=94=E2=80=94=20tag=20=E5=8A=AB=E6=8C=81=20/=20-D=20?= =?UTF-8?q?=E5=88=B0=E4=B8=8D=E4=BA=86=E8=B0=83=E7=94=A8=E6=96=B9=20/=207?= =?UTF-8?q?=20=E6=9D=A1=20Low?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 🔴 handle_wt 用裸短名解析 ref,同名 tag 会劫持证据和恢复句柄 `git rev-parse decoy` 在有同名 tag 时取的是 **tag**。于是证据和本轮刚加的恢复句柄 都会描述错误的 commit —— 为了让 -D 可恢复而加的东西,恰恰在最需要它的场景里指错。 改成传完整 ref(`$branch` 本来就是 refs/heads/)。 实测:分支 decoy=6f3da53、同名 tag 指向 main(5549b8b)。 裸短名解析出 5549b8b,完整 ref 解析出 6f3da53;修复后报告给出 **squash-PR#42** (6f3da53 的 PR),不是 tag 指向那个 commit 的 PR。 ## 🟠 -D 到不了它的主要调用方 —— 这是「修的东西到不了调用方」第三次 run.md:78 与 review-contract.md:54 调 safe-cleanup 时**没带 --squash-merged**, 而 run.md:75 合并用的是 merge-pr --squash。也就是说在 **FU-2 当初就是为之立案的**那种 squash 仓库里,自动化调用方仍然什么都清不掉;加了成本闸门之后连候选都不打印了。 前两次是 status.md(第一次)和 status.md 的纪律段(第二次)。同一个盲区第三次: 改完脚本没回头看谁在调它。 ## 7 条 Low - 恢复句柄没 shell-quote:`feat/x$(id)` 是合法 refname,打出来粘贴即执行命令替换。 改用 printf %q。实测输出 `git branch feat/x\$\(id\) 6f3da53`。 - `--integration ""` 过了元数与 flag 两道检查,空值落进自动探测**默默猜 main** —— 正是拒绝未知参数要防的那个「不可恢复」后果。现在空值也拒。 - 1b 迭代 `%(refname:short)`,同名 tag 下会拿到 `heads/x`,既匹配不上 is_protected 也匹配不上 is_in_worktree,受保护分支被列成候选。改成迭代 `%(refname)` 再剥前缀。 - protected 分支静默 continue,加宽边界集后 `integration-tests` 这类永久不可清理且零理由。 现在 dry-run 打 `KEEP (protected by name/pattern)`,但**只报模式保护** —— 集成分支和当前分支是结构性保护,每轮都说是噪音。 - TOCTOU:证据在第一个循环收、删在第二个循环,25 个分支约 20 秒窗口, 而 pilot 自己的教条意味着并发 agent 正在这些分支上提交。删之前重新取证,不一致就 SKIP。 - `--help` 写死行号第二次漂移(这次越界 4 行,把 set -euo 都打出来)。 改成读到哨兵注释为止,以后加头注释不会再漂。 - 「squash 仓库里上一节 **ALWAYS** (none)」被脚本自己的输出证伪 —— 处于或落后于集成分支 tip 的零提交废弃分支仍会被列出。README/SKILL/git-safety 三处同一句一并改成「通常」。 ## status.md 补上普通 dry-run 现在会打印的 `(not checked — one gh API call per branch…)`, 并写明那是**没查**不是**没有**。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- plugins/pilot/skills/pilot/README.md | 2 +- plugins/pilot/skills/pilot/SKILL.md | 2 +- plugins/pilot/skills/pilot/phases/run.md | 2 +- plugins/pilot/skills/pilot/phases/status.md | 3 ++ .../skills/pilot/reference/git-safety.md | 2 +- .../skills/pilot/reference/review-contract.md | 2 +- .../skills/pilot/scripts/safe-cleanup.sh | 44 ++++++++++++++----- 7 files changed, 42 insertions(+), 15 deletions(-) diff --git a/plugins/pilot/skills/pilot/README.md b/plugins/pilot/skills/pilot/README.md index 166ca32..096abee 100644 --- a/plugins/pilot/skills/pilot/README.md +++ b/plugins/pilot/skills/pilot/README.md @@ -17,7 +17,7 @@ pilot doctor # 自检本地就绪度（config/docs/分支/gh/hook）——** 把「有经验程序员的默认」固化成流程，且**危险动作确定性可控**： - **安全清理**：只删「已合并进集成分支 + 干净」的分支/worktree；默认只 `git branch -d`；默认 dry-run，`--apply` 才动手；护住主干/集成/当前/protected/脏 worktree。逻辑全在 `scripts/safe-cleanup.sh`，不靠模型临场判断。 - **squash-merge 仓库**里 `git branch --merged` 恒返回 0（squash 重写补丁，原 commit 不是集成分支的祖先），此时加 `--squash-merged`：它按 **commit** 向 GitHub 查「哪个已合并 PR 引入了这个提交」，拿到证据才用 `-D`；查不到证据、没装 gh、没登录 —— 一律保留。 + **squash-merge 仓库**里 `git branch --merged` 通常返回 0（squash 重写补丁，原 commit 不是集成分支的祖先；零提交的废弃分支仍会被它列出），此时加 `--squash-merged`：它按 **commit** 向 GitHub 查「哪个已合并 PR 引入了这个提交」，拿到证据才用 `-D`；查不到证据、没装 gh、没登录 —— 一律保留。 - **PR 纪律**：绝不 `git add -A`；绝不直推/直合主干；一个 task = 一个分支 = 一个 worktree = 一个 PR；PR 前必自测 + 对抗式 review。 - **外部 review 回路（已生产验证）**：pilot **不自评 PR**——开好 PR 后只盯自己 PR 的状态（`scripts/pr-monitor.sh --pr --wait-for-verdict`，内置 3–5 分钟轮询与 **30 分钟硬上限**；只有评审 commit == 当前 head 才算裁决）。裁决由**外部评审服务**给出，契约见 `reference/review-contract.md`：排队 5–10 分钟、评审 5–10 分钟，通常 20 分钟内出 `APPROVED`/`CHANGES_REQUESTED`（超大 PR 例外）。推新 commit 自动触发再评审。**那个服务是什么、装在哪、覆盖哪些仓库，pilot 一概不知也不启动**——只依赖这份契约。 - **pilot 是入口，配套能力由它安排**：飞书 / Notion 等文档源是 pilot 的**配套 skill**——该不该装、装哪个、装到全局还是项目级、装完怎么验证，由 pilot 负责讲清楚和安排。**但「入口」是编排责任，不是运行时依赖**：pilot 自己不 import、不启动任何文档源，只探测能力是否存在，没有就说明缺什么、给出装法，然后降级继续干活。契约与实测过的安装命令见 `reference/doc-sources.md`。 diff --git a/plugins/pilot/skills/pilot/SKILL.md b/plugins/pilot/skills/pilot/SKILL.md index 7110334..cd1b5a4 100644 --- a/plugins/pilot/skills/pilot/SKILL.md +++ b/plugins/pilot/skills/pilot/SKILL.md @@ -46,7 +46,7 @@ pilot doctor # 自检本地就绪度（config/docs/分支/gh/hook）——** 1. **绝不 `git add -A` / `git add .`**。只 `git-guard.sh add <显式路径>`（裸 `git add -A` 会被 git-guard 拒绝）。理由：`-A` 会把未确认是否该跟踪的文件（密钥、`.env`、构建产物、临时文件）一起提交，是最危险的日常动作。提交前先 `git status` 看清，逐一列出要提交的路径。 2. **绝不直接 push 到主干（main/master），绝不直接合并自己的 PR 到主干**。push 走 `git-guard.sh push`、**开 PR 走 `git-guard.sh pr-create`**(先 `preflight.sh run` 让本仓库的检查真跑过)、合并走 `git-guard.sh merge-pr --integration `（推主干 / 合并 base≠集成分支都会被硬拒绝）。所有代码变更走：feature 分支 → PR → review → 合并到**集成分支**（默认 `preview`，见 `.pilot.yml`）。主干只由集成分支经受控流程进入。**单主干仓库**（没有集成分支，PR 直接开向 `main`）加 `--allow-trunk`——它**不是绕过**：仍要求该分支的 GitHub 保护规则要求审批、且这个 PR 已经 `APPROVED`，读不到保护规则就拒绝（fail-closed）。 3. **一个 task = 一个分支 = 一个（可选）worktree = 一个 PR**。不在一个分支里顺手做别的 task。 -4. **删除分支默认只用 `git branch -d`**；删除只针对「已合并 + 干净」的分支/worktree；一切经 `scripts/safe-cleanup.sh`，默认 dry-run。**唯一的 `-D` 例外**：squash-merge 仓库里 `git branch --merged` 恒为空，此时 `--squash-merged` 会按 commit 向 GitHub 核实「哪个已合并 PR 引入了它」，**有服务端证据才删**（详见 `reference/git-safety.md`）。 +4. **删除分支默认只用 `git branch -d`**；删除只针对「已合并 + 干净」的分支/worktree；一切经 `scripts/safe-cleanup.sh`，默认 dry-run。**唯一的 `-D` 例外**：squash-merge 仓库里 `git branch --merged` 通常为空，此时 `--squash-merged` 会按 commit 向 GitHub 核实「哪个已合并 PR 引入了它」，**有服务端证据才删**（详见 `reference/git-safety.md`）。 5. **PR 之前必须自审 + 对抗 review**（怎么审见 `reference/pr-quality.md`，**审几轮由 `scripts/grade-change.sh` 机械定级，见 `reference/pre-pr-review.md`**——A/B 级 3 轮，不是作者自己说了算）。没过 review 的代码不进 PR，没 approve 的 PR 不合并。 6. **状态即文档**。每推进一步都更新 `docs/agent/tasks.md` 与 `docs/agent/progress.md`；宁可慢，不可让文档与仓库真实状态脱节。 7. **无人值守时不猜产品决策**。遇到影响产品方向/验收/架构的未知，把相关 task 标 `BLOCKED` 并记录待决问题，继续做不受影响的 task；绝不擅自替用户拍板。 diff --git a/plugins/pilot/skills/pilot/phases/run.md b/plugins/pilot/skills/pilot/phases/run.md index a36fa77..deda5b4 100644 --- a/plugins/pilot/skills/pilot/phases/run.md +++ b/plugins/pilot/skills/pilot/phases/run.md @@ -75,7 +75,7 @@ bash /scripts/check-docs.sh --docs-dir --strict `bash /scripts/git-guard.sh merge-pr --integration --squash` （git-guard 会先校验 PR base == `integration_branch`，base 是主干或其它分支会被拒绝；**不加 `--delete-branch`**——远程分支删除统一交给 §合并后的 safe-cleanup，受 `allow_remote_cleanup` 与 dirty-worktree 检查约束）。 合并后：把对应 Task 在 `tasks.md` 标 `DONE`、更新 `progress.md`，运行 - `bash /scripts/safe-cleanup.sh --integration [--protect ""] [--remote-name ] --apply` + `bash /scripts/safe-cleanup.sh --squash-merged --integration [--protect ""] [--remote-name ] --apply` 清掉本地已合并分支/worktree（**必须显式带上与 `.pilot.yml` 一致的 `--integration`/`--protect`/`--remote-name`，不要依赖脚本猜默认值**；要连带删远程，且 `allow_remote_cleanup: true` 时，再加 `--remote`）。**做完回到主循环顶端,继续下一项。** - **`decision=CHANGES_REQUESTED`** → 读全部 review 意见（`gh pr view --comments`），**先做中立 triage（见 `reference/review-triage.md`）**：装上本仓库业务上下文（CLAUDE.md / docs/agent / 领域文档），把每条意见分成 A 该修 / B 不重要 / C 缺业务上下文判错了 / D 过激 nitpick。外部评审是独立的、没有业务背景,你有——**既不盲改也不盲拒**。 - **A（+trivial 的 D）** → 在该 PR 分支修复 → 自测 → 自审 → `git-guard.sh add <显式路径>` + commit + `git-guard.sh push `（推新 commit 自动触发再评审）。 diff --git a/plugins/pilot/skills/pilot/phases/status.md b/plugins/pilot/skills/pilot/phases/status.md index f101095..869d38d 100644 --- a/plugins/pilot/skills/pilot/phases/status.md +++ b/plugins/pilot/skills/pilot/phases/status.md @@ -22,6 +22,9 @@ bash /scripts/safe-cleanup.sh --integration [--protect ""] ``` （**dry-run**，只打印 would-delete / would-remove / KEEP）。把结果原样呈现。 + 这条命令**不带** `--squash-merged`，所以「Squash-merged local branches」一节会打印 + `(not checked — one gh API call per branch; pass --squash-merged to check)` —— + 那是**没查**，不是**没有**，别当成「干净」。 - **本仓库用 squash 合并时必看**：`git branch --merged` 在 squash 仓库里**恒返回 0**， 上面那条命令的「Local merged branches」会永远是 `(none)`。脚本会在 「Squash-merged local branches」一节把候选列出来（每条附已合并的 PR 号）， diff --git a/plugins/pilot/skills/pilot/reference/git-safety.md b/plugins/pilot/skills/pilot/reference/git-safety.md index c8924f1..455ec1f 100644 --- a/plugins/pilot/skills/pilot/reference/git-safety.md +++ b/plugins/pilot/skills/pilot/reference/git-safety.md @@ -26,7 +26,7 @@ ## 删除（清理） - **删本地分支默认只用 `git branch -d`**。`-d` 会拒绝删除未合并分支，是安全网；`-D` 强删会丢未合并工作。 -- **唯一的 `-D` 例外：squash-merge 仓库。** 那里 `git branch --merged` **恒返回 0** —— squash 重写补丁， +- **唯一的 `-D` 例外：squash-merge 仓库。** 那里 `git branch --merged` **通常返回 0** —— squash 重写补丁， 原 commit 不是集成分支的祖先。实测本仓库 28 个分支、`git branch --merged main` 返回 0， 于是这个脚本在自己家里**什么都清理不了**，人只能手工 `-D`，比脚本存在还糟。 所以 `safe-cleanup.sh --squash-merged` 引入第二种**服务端**证据：GitHub 的 diff --git a/plugins/pilot/skills/pilot/reference/review-contract.md b/plugins/pilot/skills/pilot/reference/review-contract.md index 1de2d0f..e30565c 100644 --- a/plugins/pilot/skills/pilot/reference/review-contract.md +++ b/plugins/pilot/skills/pilot/reference/review-contract.md @@ -51,7 +51,7 @@ pilot **不裁决自己的 PR**。它依赖一个**外部评审服务**,并且 └─ CHANGES_REQUESTED → 中立 triage → 修 → 推 → 回到「等回执」 ``` -- **`APPROVED`** → `git-guard.sh merge-pr --integration --squash`,然后 `safe-cleanup.sh`。 +- **`APPROVED`** → `git-guard.sh merge-pr --integration --squash`,然后 `safe-cleanup.sh --squash-merged`。 - **`CHANGES_REQUESTED`** → 先按 [`review-triage.md`](review-triage.md) 做中立裁决(该改的改 / 判错的 回评论讲清业务理由 / 不阻塞的记进 [`followup-ledger.md`](followup-ledger.md)),修完推上去 **自动触发下一轮评审**,回到等回执。 diff --git a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh index 162570f..3cb5e01 100755 --- a/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh +++ b/plugins/pilot/skills/pilot/scripts/safe-cleanup.sh @@ -40,6 +40,7 @@ # Usage: # safe-cleanup.sh [--integration ] [--protect "a,b,c"] [--apply] [--squash-merged] # [--remote] [--remote-name origin] +# ---8<--- end of header ---8<--- set -euo pipefail integration="" @@ -60,7 +61,10 @@ remote_name="origin" # silent-swallow class B2 exists to close. need_val() { [ "$2" -ge 2 ] || { echo "safe-cleanup: '$1' requires a value" >&2; exit 2; } - case "$3" in -*) echo "safe-cleanup: '$1' requires a value, got flag '$3'" >&2; exit 2 ;; esac + case "$3" in + -*) echo "safe-cleanup: '$1' requires a value, got flag '$3'" >&2; exit 2 ;; + "") echo "safe-cleanup: '$1' requires a non-empty value" >&2; exit 2 ;; + esac } while [ $# -gt 0 ]; do case "$1" in @@ -70,7 +74,7 @@ while [ $# -gt 0 ]; do --apply) apply=1; shift ;; --squash-merged) squash_merged=1; shift ;; --remote) do_remote=1; shift ;; - -h|--help) sed -n '2,46p' "$0"; exit 0 ;; + -h|--help) sed -n '2,/^# ---8<--- end of header ---8<---$/p' "$0" | grep '^#'; exit 0 ;; *) echo "safe-cleanup: unknown argument '$1'" >&2 echo " usage: safe-cleanup.sh [--integration ] [--protect \"a,b\"] [--apply] [--squash-merged] [--remote] [--remote-name ]" >&2 exit 2 ;; @@ -232,8 +236,8 @@ echo "## Squash-merged local branches" # confusion B3 exists to prevent. if [ "$squash_merged" != "1" ]; then echo " (not checked — one gh API call per branch; pass --squash-merged to check)" - echo " NB: in a squash-merge repo the section above is ALWAYS (none), so cleanable" - echo " branches would appear HERE, not there." + echo " NB: in a squash-merge repo the section above is USUALLY (none) — squashed work is" + echo " not an ancestor — so cleanable branches usually appear HERE, not there." else gh_init sq_names=() @@ -241,7 +245,18 @@ sq_prs=() sq_errors=0 while IFS= read -r b; do [ -z "$b" ] && continue - is_protected "$b" && continue + # Say WHY a branch never appears. Silently skipping protected names made e.g. `integration-tests` + # permanently uncleanable with zero visible reason once the boundary set widened. + if is_protected "$b"; then + # Only surface PATTERN-protected names. The integration branch and the branch you are standing + # on are structurally protected and saying so every run is noise; what needed a visible reason + # is the third kind — e.g. `integration-tests` swept up by the widened boundary set, which + # otherwise vanishes from every section with nothing explaining why. + if [ "$apply" != "1" ] && [ "$b" != "$integration" ] && [ "$b" != "$current_branch" ]; then + echo " KEEP (protected by name/pattern) $b" + fi + continue + fi is_in_worktree "$b" && continue # Anything git already calls merged was handled above. git branch --merged "$integration" 2>/dev/null | sed 's/^[* +] *//' | grep -Fqx "$b" && continue @@ -255,7 +270,7 @@ while IFS= read -r b; do if [ "$pr" = "ERR" ]; then sq_errors=$((sq_errors + 1)); continue; fi [ -z "$pr" ] && continue sq_names+=("$b"); sq_prs+=("$pr") -done < <(git branch --format='%(refname:short)' 2>/dev/null) +done < <(git branch --format='%(refname)' 2>/dev/null | sed 's#^refs/heads/##') if [ "${#sq_names[@]}" -eq 0 ]; then if [ "$gh_state" = "unavailable" ]; then @@ -274,9 +289,15 @@ else # Capture the sha FIRST and print it ourselves: git's "Deleted branch X (was abc1234)." is # the ONLY handle for recovering a -D'd branch, and the earlier version sent it to # /dev/null. Redirect git's stdout to keep the report structured, but surface the sha. + # Evidence was gathered in the first loop; ~0.8s per gh call means a 25-branch run leaves a + # ~20s window in which a concurrent agent could commit to one of these branches. `-D` skips + # the recheck that makes section 1's `-d` immune, so re-confirm the tip before destroying it. sha_short="$(git rev-parse --short "$b" 2>/dev/null || echo unknown)" + if [ "$(merged_pr_for "$b")" != "$pr" ]; then + echo " SKIP (tip changed since evidence was collected) $b"; i=$((i + 1)); continue + fi if git branch -D -- "$b" >/dev/null 2>&1; then - echo " deleted $b (merged via PR #$pr) was=$sha_short → restore: git branch $b $sha_short" + echo " deleted $b (merged via PR #$pr) was=$sha_short → restore: git branch $(printf '%q' "$b") $sha_short" else echo " SKIP (delete failed) $b"; fi else echo " would delete $b (merged via PR #$pr)" @@ -333,7 +354,10 @@ handle_wt() { echo " KEEP (not merged per git; pass --squash-merged to also check for a squash merge) $path [$short]" return else - wt_pr="$(merged_pr_for "$short")" + # Resolve via the FULL ref, never the bare short name: with a tag of the same name, + # `git rev-parse decoy` picks the TAG, so evidence — and the recovery handle added this + # round — would describe the wrong commit. `$branch` is already refs/heads/ here. + wt_pr="$(merged_pr_for "${branch:-$short}")" if [ "$wt_pr" = "ERR" ]; then echo " KEEP (could not verify — lookup failed, NOT 'unmerged') $path [$short]" return @@ -351,9 +375,9 @@ handle_wt() { if [ "$via" = "git" ]; then git branch -d -- "$short" 2>/dev/null && echo " + deleted branch $short" || true else - local wsha; wsha="$(git rev-parse --short "$short" 2>/dev/null || echo unknown)" + local wsha; wsha="$(git rev-parse --short "${branch:-$short}" 2>/dev/null || echo unknown)" git branch -D -- "$short" >/dev/null 2>&1 \ - && echo " + deleted branch $short ($via) was=$wsha → restore: git branch $short $wsha" || true + && echo " + deleted branch $short ($via) was=$wsha → restore: git branch $(printf '%q' "$short") $wsha" || true fi else echo " SKIP (remove failed) $path [$short]" From 7739473f62f75f5ed92d269853af9c33be0a562b Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Wed, 5 Aug 2026 23:48:39 +0700 Subject: [PATCH 08/12] =?UTF-8?q?fix(pilot):=20=E7=AC=AC=E4=BA=94=E8=BD=AE?= =?UTF-8?q?=20=E2=80=94=E2=80=94=20=E4=B8=A4=E4=B8=AA=20-D=20=E7=AB=99?= =?UTF-8?q?=E7=82=B9=E5=90=88=E6=88=90=E4=B8=80=E4=B8=AA=20CAS=20=E5=88=A0?= =?UTF-8?q?=E9=99=A4,=E4=B8=8D=E5=86=8D=E5=90=84=E4=BF=AE=E4=B8=80?= =?UTF-8?q?=E5=8D=8A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 第四轮指出:上一个 commit【亲手引入了它声称要修的 tag 劫持】—— %(refname:short) 改成 %(refname)|sed 之后,消歧过的名字变回裸名, 而 git 解析裸名时 refs/tags 优先于 refs/heads。 按评审的结构性建议做:抽 destroy_branch,两个 -D 站点共用。 ## destroy_branch