From a299e20ac0e21a2daf4b71239b30f04e8837766d Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Wed, 5 Aug 2026 23:41:42 +0700 Subject: [PATCH 1/5] =?UTF-8?q?feat(pilot):=20FU-5=20=E2=80=94=E2=80=94=20?= =?UTF-8?q?=E8=B5=B7=E8=B7=91=E9=97=A8=E7=A6=81=E6=94=AF=E6=8C=81=E4=BB=93?= =?UTF-8?q?=E5=BA=93=E5=B7=B2=E6=9C=89=E7=9A=84=E8=A7=84=E5=88=92=E6=BA=90?= =?UTF-8?q?=20(v1.4.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 门禁只认 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/ 搭适配层绕过去的,这次根治。 ## 改法 .pilot.yml 可声明 planning_requires:,门禁就改查这些路径。判据一个字没松: 每条路径都要真有内容(目录 = 底下至少一个够实质的文件),空目录/只有占位符 的文件照样判 NOT ready。换的是【查哪里】,不是【要不要查】。 脚本自己读 .pilot.yml(两种 YAML 写法都认),不依赖调用方记得传 flag —— 理由和 git-guard.sh 的 self-contained 保护名单一样:run.md 分步执行, 传参会在步骤之间蒸发。这里还多一层:忘传的后果正好就是这条 bug 本身 (退回七件套 → 误判未就绪)。--planning-requires 仍可覆盖。 ## 不能拿它关掉门禁 `.` / `..` / `/` / 绝对路径 / 通配符 / 空列表 —— 全部 exit 2 拒绝, 理由和拒绝 PILOT_DOC_MIN_BYTES=0 一样:那是把门禁关了,不是配置它。 其中通配符那条藏着一个真 bug:`for p in $planning_requires` 分词的同时 也会做 glob 展开,而且发生在循环体之前 —— `backlog/*` 曾经静默变成它 匹配到的九个路径,检查根本看不到那个 `*`(实测 ok=7/9)。加 set -f 才真拒得掉。 ## --no-config 与 CI 的交互(差点埋雷) scripts/ci/check-docs-gate.sh 在仓库根跑 `--docs-dir ` 测门禁自身。 脚本一旦会读 .pilot.yml,这个仓库将来只要声明了 planning_requires, 那些断言就会【静默改去查 backlog/】,一边继续打印 ok 一边测着别的东西。 加 --no-config 显式隔离,并补一条断言把这件事钉死:谁把 flag 拿掉, 红的是那条断言,而不是悄悄失效。 ## 验证 check-docs-gate.sh 从 8 条断言加到 16 条,全绿: - 老行为逐条回归(原样模板拒 / 阈值非法拒 / 空目录拒 / 填好的放行) - 新增:替代源有内容→放行、全空→拒、路径不存在→拒 - 新增:`.` / `/` / 绝对路径 / 通配符 → 全部 exit 2 - 新增:--no-config 确实隔离仓库声明 另外实测两种 YAML 写法(行内数组 + 块列表)、未声明时退回七件套。 Brood 自己的 .pilot.yml 【这次不改】—— 能力就绪不等于该立刻切换, 那是第二个决定。#45 的教训就是一个 PR 装了四件事。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- plugins/pilot/.claude-plugin/plugin.json | 2 +- plugins/pilot/skills/pilot/SKILL.md | 10 +- plugins/pilot/skills/pilot/phases/run.md | 3 +- .../pilot/skills/pilot/scripts/check-docs.sh | 203 ++++++++++++++++-- .../skills/pilot/templates/pilot.example.yml | 12 ++ scripts/ci/check-docs-gate.sh | 48 ++++- 6 files changed, 249 insertions(+), 29 deletions(-) diff --git a/plugins/pilot/.claude-plugin/plugin.json b/plugins/pilot/.claude-plugin/plugin.json index 3d55bb4..2c97559 100644 --- a/plugins/pilot/.claude-plugin/plugin.json +++ b/plugins/pilot/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "pilot", - "version": "1.3.0", + "version": "1.4.0", "description": "仓库级开发操作系统:status 盘点+安全清理 → plan 三级规划 → run 无人值守交付循环(跑到交付为止,起跑前强制文档齐全门禁 + 默认设 /goal 停止闸);git-guard 危险动作防护(best-effort) + safe-cleanup 已合并分支清理 + PR 开出后按 reference/review-contract.md 盯外部评审裁决(不启动、不感知任何评审后端)。", "author": { "name": "AAStarCommunity" diff --git a/plugins/pilot/skills/pilot/SKILL.md b/plugins/pilot/skills/pilot/SKILL.md index e5e8117..58ef462 100644 --- a/plugins/pilot/skills/pilot/SKILL.md +++ b/plugins/pilot/skills/pilot/SKILL.md @@ -1,6 +1,6 @@ --- name: pilot -version: 1.3.0 +version: 1.4.0 description: 仓库级开发操作系统。三阶段驱动一个仓库从「盘点 → 规划 → 持续开发」全流程。status=汇报进展+安全清理已合并分支/worktree；plan=建立/汇报 Milestone→Feature→Task 三级规划；run=先交出一条填好的 /goal 交付契约(说清怎么用 plan 的文档、怎么验证、PR 由外部评审服务裁决要怎么等、什么时候才算交付),再照它连续迭代做到交付;起跑前强制检查规划文档齐全。当用户说 pilot / 整理仓库 / 汇报进展 / 清理分支 / 规划里程碑 / 持续开发 / 跑通宵开发时使用。 allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Task, TodoWrite, Monitor, ScheduleWakeup --- @@ -64,6 +64,8 @@ protect_patterns: [release, hotfix] # 额外保护的分支前缀 remote: origin allow_remote_cleanup: false # 删除远程已合并分支需显式置 true docs_dir: docs/agent # 规划/运行态文档目录 +planning_requires: # 可选。规划已在别处时,声明它的本地路径,起跑门禁改查这些 + - backlog/tasks #（不声明 = 老行为:查 docs_dir 下那七个固定文件名) ``` 读取方式：用 Read 工具读该文件，把值作为 flag 传给脚本（脚本本身不解析 YAML，保持简单确定）。文件不存在时用上表默认值，并提示用户运行 `pilot doctor` 生成。 @@ -79,9 +81,13 @@ docs_dir: docs/agent # 规划/运行态文档目录 2b. **配置迁移硬检查**：若旧文件 `.repo-pilot.yml` 存在—— - 只有旧文件、无 `.pilot.yml`：红字提示「`.repo-pilot.yml` 已弃用，运行 `git mv .repo-pilot.yml .pilot.yml`」；在迁移前 `run`/`status` 会读旧文件兜底。 - **两个文件都在、且 `integration_branch` 不一致：`FAIL`（阻断）**——因为哪个生效不确定、错的那个可能把 PR 直合进主干。要求用户先删掉/合并旧文件再继续，`doctor` 不擅自改。 -3. **规划文档齐全度**（`run` 无人值守的硬前提）：`bash /scripts/check-docs.sh --docs-dir --strict`。 +3. **规划层齐全度**（`run` 无人值守的硬前提）：`bash /scripts/check-docs.sh --docs-dir --strict`。 报 MISSING/EMPTY 就照实列出并建议 `pilot plan` 补齐——`run` 会在同一道门禁上 fail-closed 拒跑， 在这里先看见比半夜被拦住强。（脚本会识别「文件在但还是原样模板」：占位符没填等于没答。） + **看输出的 `source=` 字段**：`dir=` 表示查的是 docs_dir 下那七个文件；`source=planning_requires(...)` 表示 + 这个仓库在 `.pilot.yml` 里声明了别的规划源，查的是那些路径。**规划已经在 backlog.md / issue tracker / + 别的工具里的仓库,报 NOT ready 时不要建议把规划重抄进七件套**——那违反 plan.md §A.3(已有规划不要重复造)。 + 正确建议是声明 `planning_requires:` 指向真正的规划源。 4. 是否有集成分支（`git show-ref refs/heads/`）；无则提示先建。 5. `gh auth status` 是否可用（PR 流程需要）。 5b. **git hook 是否真的在生效**（别假设 commit 有保护）：`bash /scripts/check-hooks.sh`。常见坑：`core.hooksPath` 指到**另一个 clone** 的 hooks 目录 → pre-commit 密钥扫描根本没跑,commit 裸奔却无人察觉。报 `BYPASSED` 就红着提示,并说明「**不要自动切回 `.githooks`**——扫描器有历史误报会让每次 commit 卡死,得先给已知误报加 baseline/allowlist 降噪,再手动开钩子」。只报告,不擅自 rewire。 diff --git a/plugins/pilot/skills/pilot/phases/run.md b/plugins/pilot/skills/pilot/phases/run.md index a36fa77..090441a 100644 --- a/plugins/pilot/skills/pilot/phases/run.md +++ b/plugins/pilot/skills/pilot/phases/run.md @@ -14,8 +14,9 @@ task → 等评审 → 合并 → 再挑下一个，**直到 §3 的交付条件 bash /scripts/check-docs.sh --docs-dir --strict ``` -- `rc=0` → 七件套齐全且**已填写**（脚本会识别「还是原样模板」——占位符没填等于没答，比没有更危险），继续 0b。 +- `rc=0` → 规划层齐全且**已填写**（脚本会识别「还是原样模板」——占位符没填等于没答，比没有更危险），继续 0b。 - **`rc≠0` 一律停下**（`1`=有缺口，`2`=用法/参数错，例如 `PILOT_DOC_MIN_BYTES` 不是正整数）。把脚本输出原样报给用户；`rc=1` 时建议 `pilot plan` 补齐。**不要把非零当成「大概能跑」**。 +- **规划已经在别处的仓库**（backlog.md / issue tracker / 别的工具）：不要为了过门禁去把已有规划重抄一份到七件套里——那正违反 plan.md §A.3。在 `.pilot.yml` 里声明 `planning_requires:` 指向那个规划源的本地路径，脚本会自动读到并改查那些路径（判据不变，仍要求真有内容）。脚本输出的 `source=` 字段会写明这一轮查的是哪一种，照它汇报。 **不许带着缺口开跑**，也不许自己动手把文档编出来替用户拍板（违反 SKILL.md 硬约束 7）。 - 用户明确说「有人盯着、先跑起来」→ 才可降级 `--minimal`（只要 roadmap+tasks+progress）， 并在汇报里写明**这是降级运行、缺哪几份文档**。降级是用户的决定，不是你的。 diff --git a/plugins/pilot/skills/pilot/scripts/check-docs.sh b/plugins/pilot/skills/pilot/scripts/check-docs.sh index 80714d4..2276e9d 100755 --- a/plugins/pilot/skills/pilot/scripts/check-docs.sh +++ b/plugins/pilot/skills/pilot/scripts/check-docs.sh @@ -2,6 +2,7 @@ # check-docs.sh — gate: is the planning layer complete enough to run UNATTENDED? # # bash scripts/check-docs.sh [--docs-dir docs/agent] [--strict|--minimal] +# [--planning-requires ] [--no-config] # # Unattended delivery means nobody is around to answer "what did you mean here?". # Every gap in the planning layer becomes a guess the model makes alone at 3am, so this @@ -16,7 +17,19 @@ # (`<...>` angle-bracket slots the templates ship with). Shipping a template with the # slots unfilled is worse than no doc — it reads as answered when it isn't. # +# TWO SHAPES OF PLANNING LAYER +# Default: the seven `docs_dir/*.md` files `pilot plan` produces. +# Alternative: a repo that already plans in backlog.md / an issue tracker / anything else +# declares `planning_requires:` in `.pilot.yml` (or passes --planning-requires), and THOSE +# paths are checked instead (`--no-config` ignores the declaration and checks the seven docs +# regardless — for testing the gate itself). Rationale: the fixed seven filenames made the gate unable to +# see an equivalent — often richer — planning source, so `run` fail-closed on repos that +# were in fact ready, while plan.md §A.3 simultaneously says "already planned → do not +# re-create". Both rules could not be obeyed at once. A path is checked with the SAME +# content test as a doc; a directory passes when at least one file under it passes. +# # Exit: 0 = ready, 1 = gaps found (list printed), 2 = usage error. +# ---8<--- end of help set -uo pipefail docs_dir="docs/agent" @@ -50,16 +63,65 @@ if [ "$MIN_BYTES" -eq 0 ]; then exit 2 fi +planning_requires="" +planning_src="flag" +read_config=1 + while [ $# -gt 0 ]; do case "$1" in --docs-dir) [ $# -ge 2 ] || { echo "ERROR: --docs-dir needs a path" >&2; exit 2; }; docs_dir="$2"; shift 2 ;; + --planning-requires) + [ $# -ge 2 ] || { echo "ERROR: --planning-requires needs a comma-separated path list" >&2; exit 2; } + planning_requires="$2"; shift 2 ;; + # Test/inspection escape hatch: check ONLY the seven docs under --docs-dir, ignoring whatever + # the repo declares. Needed because reading .pilot.yml would otherwise silently hijack a + # deliberate `--docs-dir ` — scripts/ci/check-docs-gate.sh runs exactly that shape from + # the repo root, and without this flag every one of its assertions would quietly stop testing + # what it says it tests the moment this repo declared planning_requires. + --no-config) read_config=0; shift ;; --strict) mode="strict"; shift ;; --minimal) mode="minimal"; shift ;; - -h|--help) sed -n '2,20p' "$0"; exit 0 ;; + # Print to the sentinel below rather than a hardcoded line range: a range drifts the moment + # anyone adds a line to the header, and it drifts SILENTLY (measured on git-guard.sh: a 4-line + # overrun printed `set -euo pipefail` as if it were help text). + -h|--help) sed -n '2,/^# ---8<--- end of help/p' "$0" | grep -v '^# ---8<---'; exit 0 ;; *) echo "ERROR: unknown arg: $1" >&2; exit 2 ;; esac done +# Read `planning_requires:` from .pilot.yml when the caller did not pass it. +# +# Same reasoning as git-guard.sh's self-contained protection list: run.md executes as separate +# Bash calls that share no variables, so a caller-threaded flag silently vanishes between steps. +# Here the stakes are the mirror image of git-guard's — forgetting the flag makes this gate MORE +# strict (it falls back to the seven filenames and refuses to run), which is safe but is exactly +# the false "NOT ready" this feature exists to remove. So read the config directly; the flag stays +# available as an override. +# +# Accepts both YAML shapes: planning_requires: [a, b] and a block list of `- a` lines. +if [ -z "$planning_requires" ] && [ "$read_config" = "1" ]; then + _top="$(git rev-parse --show-toplevel 2>/dev/null || echo .)" + for _f in "$_top/.pilot.yml" "$_top/.repo-pilot.yml"; do + [ -f "$_f" ] || continue + planning_requires="$(awk ' + /^planning_requires:[[:space:]]*\[/ { + line = $0; sub(/^[^[]*\[/, "", line); sub(/\].*$/, "", line); print line; exit + } + /^planning_requires:[[:space:]]*$/ { inlist = 1; next } + inlist { + # A block list ends at the first line that is not an indented `- item`. + if ($0 ~ /^[[:space:]]+-[[:space:]]*[^[:space:]]/) { + item = $0; sub(/^[[:space:]]*-[[:space:]]*/, "", item); printf "%s,", item; next + } + if ($0 ~ /^[[:space:]]*$/) next + exit + } + ' "$_f" | tr -d '"'"'"' ' | sed 's/,$//')" + [ -n "$planning_requires" ] && planning_src="$(basename "$_f")" + break + done +fi + # Ordered by the information flow in plan.md: research → acceptance → architecture+spec # → roadmap → tasks → progress. Earlier docs constrain later ones, so report them in order. STRICT_DOCS="research acceptance architecture spec roadmap tasks progress" @@ -82,38 +144,135 @@ real_content_bytes() { | wc -c | tr -d ' ' } -missing=""; empty=""; ok=0 -for d in $DOCS; do - f="$docs_dir/$d.md" - if [ ! -f "$f" ]; then - missing="$missing $d" - continue +# Does this path hold real content? A file must pass real_content_bytes itself; a directory +# passes when at least ONE file under it does. Directories are the normal case for the +# alternative source (`backlog/tasks/` holds one file per task), and requiring every file to +# pass would fail on the tracker's own scaffolding (empty archive dirs, drafts). +path_has_content() { + local p="$1" f b + if [ -f "$p" ]; then + b="$(real_content_bytes "$p")" + case "$b" in ''|*[!0-9]*) return 1 ;; esac + [ "$b" -ge "$MIN_BYTES" ] && return 0 + return 1 fi - bytes="$(real_content_bytes "$f")" - # Treat an unreadable measurement as EMPTY, never as OK. Same reasoning as the MIN_BYTES - # validation above: if the count is not a plain integer the comparison would error, the - # branch would read false, and the doc would be silently counted as filled in. - case "$bytes" in ''|*[!0-9]*) empty="$empty $d"; continue ;; esac - if [ "$bytes" -lt "$MIN_BYTES" ]; then - empty="$empty $d" - else - ok=$((ok + 1)) + if [ -d "$p" ]; then + # -print -quit stops at the first hit, so a huge tracker directory costs one file read. + while IFS= read -r f; do + b="$(real_content_bytes "$f")" + case "$b" in ''|*[!0-9]*) continue ;; esac + [ "$b" -ge "$MIN_BYTES" ] && return 0 + done </dev/null | head -200) +EOF + return 1 fi -done + return 1 +} + +missing=""; empty=""; ok=0 -total=$(echo $DOCS | wc -w | tr -d ' ') -# Echo the effective threshold on EVERY outcome, including success. PILOT_DOC_MIN_BYTES can -# weaken the gate from outside the repo; if the passing line never showed it, a loosened gate -# would leave no trace in the run's report. -echo "PILOT_DOCS: mode=$mode dir=$docs_dir min_bytes=$MIN_BYTES ok=$ok/$total" +if [ -n "$planning_requires" ]; then + # ---- alternative planning source ----------------------------------------------------------- + # This replaces the seven filenames; it does not relax the content test. Each declared path is + # held to the same MIN_BYTES / no-unfilled-`<...>`-slots bar a doc is. + # + # Refuse paths that would make the gate meaningless by matching the whole repo. `planning_requires: [.]` + # passes trivially in ANY repo with one non-empty file, which is not "configured", it is "off" — + # same reasoning as refusing PILOT_DOC_MIN_BYTES=0 above. The knob is for pointing at a real + # planning source, not for opting out of the gate. + # `set -f` is load-bearing, not tidiness. Word-splitting an unquoted $planning_requires ALSO + # glob-expands it, and that happens BEFORE the loop body — so a `backlog/*` entry silently + # became the nine paths it matched and the glob check below never saw a `*` at all (measured: + # ok=7/9 from a single declared entry). With globbing off, the entry arrives literal and the + # check can refuse it. Restored right after the split. + set -f + OLDIFS="$IFS"; IFS=',' + for p in $planning_requires; do + IFS="$OLDIFS" + # Trim whitespace only. Trailing-slash removal comes AFTER the checks below — stripping first + # turned a bare "/" into "" and it fell out of the loop as "empty list", reporting the wrong + # reason for a refusal that should name what was actually passed. + p="$(printf '%s' "$p" | sed -e 's|^[[:space:]]*||' -e 's|[[:space:]]*$||')" + [ -z "$p" ] && continue + case "$p" in + .|./|..|../|/|'~') + echo "ERROR: planning_requires entry '$p' matches the whole repo — that disables the gate rather than configuring it. Point it at the actual planning source (e.g. backlog/tasks)." >&2 + exit 2 ;; + /*|*..*) + echo "ERROR: planning_requires entry '$p' must be a repo-relative path without '..' — refusing." >&2 + exit 2 ;; + # A glob would be expanded by whoever wrote it (or not at all, becoming a literal path that + # is simply MISSING) — either way the gate would be checking something other than what the + # config appears to say. Require literal paths so the declaration means one fixed thing. + *'*'*|*'?'*|*'['*) + echo "ERROR: planning_requires entry '$p' contains a glob — pass literal paths so the declaration checks exactly what it says." >&2 + exit 2 ;; + esac + p="${p%/}" + REQ_LIST="${REQ_LIST:-} $p" + IFS=',' + done + IFS="$OLDIFS" + set +f + [ -n "${REQ_LIST:-}" ] || { echo "ERROR: planning_requires is set but resolved to an empty list — refusing (an empty list would pass unconditionally)." >&2; exit 2; } + + for p in $REQ_LIST; do + if [ ! -e "$p" ]; then + missing="$missing $p" + elif path_has_content "$p"; then + ok=$((ok + 1)) + else + empty="$empty $p" + fi + done + total=$(echo $REQ_LIST | wc -w | tr -d ' ') + echo "PILOT_DOCS: mode=$mode source=planning_requires($planning_src) min_bytes=$MIN_BYTES ok=$ok/$total" + echo " declared planning source:$REQ_LIST" +else + for d in $DOCS; do + f="$docs_dir/$d.md" + if [ ! -f "$f" ]; then + missing="$missing $d" + continue + fi + bytes="$(real_content_bytes "$f")" + # Treat an unreadable measurement as EMPTY, never as OK. Same reasoning as the MIN_BYTES + # validation above: if the count is not a plain integer the comparison would error, the + # branch would read false, and the doc would be silently counted as filled in. + case "$bytes" in ''|*[!0-9]*) empty="$empty $d"; continue ;; esac + if [ "$bytes" -lt "$MIN_BYTES" ]; then + empty="$empty $d" + else + ok=$((ok + 1)) + fi + done + total=$(echo $DOCS | wc -w | tr -d ' ') + # Echo the effective threshold on EVERY outcome, including success. PILOT_DOC_MIN_BYTES can + # weaken the gate from outside the repo; if the passing line never showed it, a loosened gate + # would leave no trace in the run's report. + echo "PILOT_DOCS: mode=$mode dir=$docs_dir min_bytes=$MIN_BYTES ok=$ok/$total" +fi if [ -z "$missing" ] && [ -z "$empty" ]; then echo "PILOT_DOCS: ready — planning layer complete, safe to run unattended." exit 0 fi +# In planning_requires mode the entries ARE paths already — prefixing them with $docs_dir would +# print a path that does not exist and send whoever reads it to the wrong place. +if [ -n "$planning_requires" ]; then + [ -n "$missing" ] && { echo " MISSING (path absent):"; for d in $missing; do echo " - $d"; done; } + [ -n "$empty" ] && { echo " EMPTY (no file with ${MIN_BYTES}B+ of real content under it):"; for d in $empty; do echo " - $d"; done; } + echo "PILOT_DOCS: NOT ready — the planning source declared in .pilot.yml is incomplete." + echo " Fix the source itself, or correct \`planning_requires:\` if it points at the wrong paths." + echo " (do NOT switch back to the seven docs just to get past this — that re-creates planning that already exists elsewhere; see plan.md §A.3)" + exit 1 +fi + [ -n "$missing" ] && { echo " MISSING (file absent):"; for d in $missing; do echo " - $docs_dir/$d.md"; done; } [ -n "$empty" ] && { echo " EMPTY (still the template / under ${MIN_BYTES}B of real content):"; for d in $empty; do echo " - $docs_dir/$d.md"; done; } echo "PILOT_DOCS: NOT ready — run \`pilot plan\` to fill these in before an unattended run." echo " (a human-supervised run can proceed with --minimal: roadmap + tasks + progress)" +echo " (already plan in backlog.md / an issue tracker? declare \`planning_requires:\` in .pilot.yml so this gate checks THAT instead — see the header of this script)" exit 1 diff --git a/plugins/pilot/skills/pilot/templates/pilot.example.yml b/plugins/pilot/skills/pilot/templates/pilot.example.yml index 11b0c79..781cf1b 100644 --- a/plugins/pilot/skills/pilot/templates/pilot.example.yml +++ b/plugins/pilot/skills/pilot/templates/pilot.example.yml @@ -10,4 +10,16 @@ remote: origin allow_remote_cleanup: false # 删除远程已合并分支需显式置 true（无人值守默认不删远程） docs_dir: docs/agent # 规划/运行态文档目录 +# 规划已经在别处（backlog.md / issue tracker / 任何工具）时，在这里声明它的本地产物路径， +# 起跑门禁就检查这些路径，而不是 docs_dir 下那七个固定文件名。不声明 = 老行为（查七件套）。 +# +# 判据不变，只是换了查哪里：每条路径都要真有内容（目录 = 底下至少一个够实质的文件）， +# 空目录 / 只有占位符的文件照样判 NOT ready。声明 `.` / 绝对路径 / 通配符会被直接拒绝 +# —— 那是把门禁关掉，不是配置它。 +# +# planning_requires: +# - backlog/tasks +# - backlog/docs +# - docs/agent/progress.md # 运行态文档仍归 docs_dir，按需一并列进来 + # PR 的裁决由外部评审服务给出，pilot 只负责盯状态（契约见 reference/review-contract.md） diff --git a/scripts/ci/check-docs-gate.sh b/scripts/ci/check-docs-gate.sh index f1267f0..f40b435 100755 --- a/scripts/ci/check-docs-gate.sh +++ b/scripts/ci/check-docs-gate.sh @@ -12,6 +12,9 @@ set -uo pipefail GATE="plugins/pilot/skills/pilot/scripts/check-docs.sh" +# Absolute form: assertion 5 must `cd` into a fixture (planning_requires entries are repo-relative +# by design), and a relative $GATE stops resolving the moment we leave the repo root. +GATE_ABS="$PWD/$GATE" TEMPLATES="plugins/pilot/skills/pilot/templates" fails=0 @@ -38,7 +41,11 @@ check() { # check fi } -run() { bash "$GATE" --docs-dir "$tmp" "$@" >/dev/null 2>&1; echo $?; } +# --no-config on EVERY call: these assertions are about the gate's own logic against a fixture +# directory, and this script runs from the repo root. Without it, the gate would read this repo's +# .pilot.yml, see `planning_requires:`, and check backlog/ instead of $tmp — every assertion below +# would keep printing "ok" while testing something else entirely. Assertion 7 pins that down. +run() { bash "$GATE" --no-config --docs-dir "$tmp" "$@" >/dev/null 2>&1; echo $?; } echo "check-docs.sh fail-closed assertions:" @@ -59,7 +66,7 @@ check "zero threshold rejected" 2 "$(PILOT_DOC_MIN_BYTES=0 run --stric # 3. Missing files must be rejected too (the gate's other half). empty="$(mktemp -d)" -check "empty docs dir rejected" 1 "$(bash "$GATE" --docs-dir "$empty" --strict >/dev/null 2>&1; echo $?)" +check "empty docs dir rejected" 1 "$(bash "$GATE" --no-config --docs-dir "$empty" --strict >/dev/null 2>&1; echo $?)" rm -rf "$empty" # 4. The gate must still PASS on genuinely filled docs — otherwise it is just broken, and a gate @@ -73,7 +80,42 @@ for d in research acceptance architecture spec roadmap tasks progress; do echo "补充说明:这一段用于验证门禁在文档确实填写之后能够正常放行,而不是一律拒绝。" } > "$filled/$d.md" done -check "filled docs accepted" 0 "$(bash "$GATE" --docs-dir "$filled" --strict >/dev/null 2>&1; echo $?)" +check "filled docs accepted" 0 "$(bash "$GATE" --no-config --docs-dir "$filled" --strict >/dev/null 2>&1; echo $?)" + +rm -rf "$filled" + +# 5. An alternative planning source (`planning_requires:`) must be held to the SAME content bar. +# It replaces WHICH paths are checked, never WHETHER they have to be real — a knob that let a +# repo declare its way past the gate would be the fail-open this whole script exists to catch. +# Run from inside the fixture (entries must be repo-relative), so the gate needs an ABS path. +alt="$(mktemp -d)"; mkdir -p "$alt/plan-src" "$alt/blank-src" +: > "$alt/blank-src/placeholder.md" +{ + echo "# 规划源" + echo "本文件包含足够的实质内容,用于验证门禁在替代规划源下能够正常放行,而不是一律拒绝。" + echo "第二段补充说明,确保真实内容超过最小字节阈值。" +} > "$alt/plan-src/tasks.md" +check "planning_requires: dir with real content accepted" 0 \ + "$(cd "$alt" && bash "$GATE_ABS" --planning-requires "plan-src" --strict >/dev/null 2>&1; echo $?)" +check "planning_requires: dir of blank files rejected" 1 \ + "$(cd "$alt" && bash "$GATE_ABS" --planning-requires "blank-src" --strict >/dev/null 2>&1; echo $?)" +check "planning_requires: absent path rejected" 1 \ + "$(cd "$alt" && bash "$GATE_ABS" --planning-requires "plan-src,nope" --strict >/dev/null 2>&1; echo $?)" +rm -rf "$alt" + +# 6. The knob must not be usable to switch the gate OFF. Each of these would otherwise pass +# unconditionally in any repo holding a single non-empty file. +check "planning_requires='.' refused" 2 "$(bash "$GATE" --planning-requires "." --strict >/dev/null 2>&1; echo $?)" +check "planning_requires='/' refused" 2 "$(bash "$GATE" --planning-requires "/" --strict >/dev/null 2>&1; echo $?)" +check "planning_requires absolute refused" 2 "$(bash "$GATE" --planning-requires "/etc" --strict >/dev/null 2>&1; echo $?)" +# A glob is refused rather than expanded: word-splitting an unquoted list ALSO globs, so `backlog/*` +# used to arrive pre-expanded as the paths it matched and the literal check never saw a `*`. +check "planning_requires glob refused" 2 "$(bash "$GATE" --planning-requires "backlog/*" --strict >/dev/null 2>&1; echo $?)" + +# 7. --no-config must actually isolate the fixture from this repo's declaration. If someone drops +# the flag from run() above, THIS is the assertion that goes red instead of the suite silently +# testing backlog/ while claiming to test pristine templates. +check "--no-config ignores repo declaration" 1 "$(bash "$GATE" --no-config --docs-dir "$tmp" --strict >/dev/null 2>&1; echo $?)" rm -rf "$filled" echo From e26452e4bc214cb1b72bd6e3748e1780a5d3263d Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Thu, 6 Aug 2026 02:12:57 +0700 Subject: [PATCH 2/5] =?UTF-8?q?fix(pilot):=20=E4=B8=A4=E6=9D=A1=20High=20?= =?UTF-8?q?=E2=80=94=E2=80=94=20=E6=8B=92=E7=BB=9D=E8=A1=A8=E5=8F=AF?= =?UTF-8?q?=E7=BB=95=E8=BF=87=20+=20=E8=A7=A3=E6=9E=90=E5=99=A8=E8=AF=BB?= =?UTF-8?q?=E4=B8=8D=E4=BA=86=E8=87=AA=E5=B7=B1=E6=96=87=E6=A1=A3=E7=9A=84?= =?UTF-8?q?=E5=86=99=E6=B3=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 1. 不再枚举「.」有多少种拼法,改成解析后比较 原来是一张字符串表 `.|./|..|../|/|~`,而 `.//` `./.` `././` `.///` `.//.` 既不是绝对路径、不含 `..`、不含通配符 —— 全部通过,随后 `${p%/}` 把它们 归一化,整个仓库就成了「规划源」。`.git` 更省事,它在任何 git 仓库里都在。 实测(完全没有规划文档的仓库): 不配置 → rc=1 ok=0/7 NOT ready planning_requires: [.//] → rc=0 ok=1/1 "ready — safe to run unattended" 改成 `cd "$p" && pwd -P` 解析后与 toplevel 比较,并拒掉 .git、拒掉解析后 落在仓库外的路径(所以用 -P,符号链接一并关掉)。一次关掉整个家族,不是 等下一个变体再补一行。 ## 2. 我自己文档的写法,我自己的解析器读不了 —— 而且静默 awk 的 key 正则是 `^planning_requires:[[:space:]]*$`,而 SKILL.md 文档的 写法带行尾注释,于是永远匹配不上、inlist 不置位、声明被【静默丢弃】、 门禁退回七件套 —— 正是这个功能立项要消灭的那个假 NOT-ready,由这个功能 自己送达。条目级注释更糟:shipped 模板那行会被粘成 `路径#注释` → MISSING。 改:awk 先剥掉 ` #…`(只剥空白开头的,路径里合法的 `#` 不受影响),key 行 和条目行都剥;并且【key 存在但解析出 0 条 → exit 2】,绝不静默回退 —— 静默回退和这个 bug 本身无法区分。 顺带把 SKILL.md 和模板里那两处写法改成解析器确实能读的形状。 ## 验证 CI 断言 16 → 28 条,全绿。新增覆盖:七种仓库根拼法 + .git / .git/refs 全部 rc=2;key 行和条目带注释都能读;行内数组带注释能读;declared-but-empty 和显式空数组都 exit 2;没有 key 时正常回退七件套(rc=1)。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- plugins/pilot/skills/pilot/SKILL.md | 5 +- .../pilot/skills/pilot/scripts/check-docs.sh | 78 ++++++++++++++++--- .../skills/pilot/templates/pilot.example.yml | 9 ++- scripts/ci/check-docs-gate.sh | 33 ++++++++ 4 files changed, 109 insertions(+), 16 deletions(-) diff --git a/plugins/pilot/skills/pilot/SKILL.md b/plugins/pilot/skills/pilot/SKILL.md index 0c5dd5d..b1b21dc 100644 --- a/plugins/pilot/skills/pilot/SKILL.md +++ b/plugins/pilot/skills/pilot/SKILL.md @@ -64,8 +64,9 @@ protect_patterns: [release, hotfix] # 额外保护的分支前缀 remote: origin allow_remote_cleanup: false # 删除远程已合并分支需显式置 true docs_dir: docs/agent # 规划/运行态文档目录 -planning_requires: # 可选。规划已在别处时,声明它的本地路径,起跑门禁改查这些 - - backlog/tasks #（不声明 = 老行为:查 docs_dir 下那七个固定文件名) +# 可选。规划已在别处时,声明它的本地路径,起跑门禁就改查这些(不声明 = 老行为:查上面那七个文件名) +planning_requires: + - backlog/tasks ``` 读取方式：用 Read 工具读该文件，把值作为 flag 传给脚本（脚本本身不解析 YAML，保持简单确定）。文件不存在时用上表默认值，并提示用户运行 `pilot doctor` 生成。 diff --git a/plugins/pilot/skills/pilot/scripts/check-docs.sh b/plugins/pilot/skills/pilot/scripts/check-docs.sh index 2276e9d..e7347da 100755 --- a/plugins/pilot/skills/pilot/scripts/check-docs.sh +++ b/plugins/pilot/skills/pilot/scripts/check-docs.sh @@ -103,21 +103,51 @@ if [ -z "$planning_requires" ] && [ "$read_config" = "1" ]; then _top="$(git rev-parse --show-toplevel 2>/dev/null || echo .)" for _f in "$_top/.pilot.yml" "$_top/.repo-pilot.yml"; do [ -f "$_f" ] || continue + # YAML permits a trailing `# comment` on the key line AND on every item. The first version of + # this parser matched `^planning_requires:[[:space:]]*$`, so the form documented in this very + # skill — `planning_requires: # 可选…` — never matched: `inlist` stayed 0, the declaration was + # dropped SILENTLY, and the gate fell back to the seven filenames. That is precisely the false + # "NOT ready" this feature exists to remove, arriving through the feature itself. Item comments + # were worse: the shipped template's `- docs/agent/progress.md # 运行态…` became the literal + # path `docs/agent/progress.md#运行态…` once whitespace was stripped. + # Strip only WHITESPACE-PRECEDED `#`, which is what YAML calls a comment, so a path that + # legitimately contains `#` survives. planning_requires="$(awk ' - /^planning_requires:[[:space:]]*\[/ { - line = $0; sub(/^[^[]*\[/, "", line); sub(/\].*$/, "", line); print line; exit + function clean(s) { + sub(/[[:space:]]+#.*$/, "", s) + gsub(/^[[:space:]]+|[[:space:]]+$/, "", s) + return s } - /^planning_requires:[[:space:]]*$/ { inlist = 1; next } + { line = $0; sub(/[[:space:]]+#.*$/, "", line) } + line ~ /^planning_requires:[[:space:]]*\[/ { + v = line; sub(/^[^[]*\[/, "", v); sub(/\].*$/, "", v) + n = split(v, a, ",") + for (i = 1; i <= n; i++) { t = clean(a[i]); if (t != "") printf "%s,", t } + exit + } + line ~ /^planning_requires:[[:space:]]*$/ { inlist = 1; next } inlist { # A block list ends at the first line that is not an indented `- item`. - if ($0 ~ /^[[:space:]]+-[[:space:]]*[^[:space:]]/) { - item = $0; sub(/^[[:space:]]*-[[:space:]]*/, "", item); printf "%s,", item; next + if (line ~ /^[[:space:]]+-[[:space:]]*[^[:space:]]/) { + item = line; sub(/^[[:space:]]*-[[:space:]]*/, "", item) + t = clean(item); if (t != "") printf "%s,", t + next } - if ($0 ~ /^[[:space:]]*$/) next + if (line ~ /^[[:space:]]*$/) next exit } - ' "$_f" | tr -d '"'"'"' ' | sed 's/,$//')" - [ -n "$planning_requires" ] && planning_src="$(basename "$_f")" + ' "$_f" | tr -d "\"'" | sed 's/,$//')" + if [ -n "$planning_requires" ]; then + planning_src="$(basename "$_f")" + elif grep -qE '^planning_requires:' "$_f" 2>/dev/null; then + # The key is THERE but nothing parsed out of it. Never fall back silently: a quiet fallback + # reports "NOT ready, go write the seven docs" to a repo that did declare its planning source, + # which is indistinguishable from the bug this feature fixes. Fail loudly instead. + echo "ERROR: $_f declares 'planning_requires:' but no usable entries parsed out of it." >&2 + echo " Expected either planning_requires: [a, b] or an indented block list of '- path' lines." >&2 + echo " Refusing to silently fall back to the default docs — that would report NOT ready for a repo that DID declare a source." >&2 + exit 2 + fi break done fi @@ -196,9 +226,6 @@ if [ -n "$planning_requires" ]; then p="$(printf '%s' "$p" | sed -e 's|^[[:space:]]*||' -e 's|[[:space:]]*$||')" [ -z "$p" ] && continue case "$p" in - .|./|..|../|/|'~') - echo "ERROR: planning_requires entry '$p' matches the whole repo — that disables the gate rather than configuring it. Point it at the actual planning source (e.g. backlog/tasks)." >&2 - exit 2 ;; /*|*..*) echo "ERROR: planning_requires entry '$p' must be a repo-relative path without '..' — refusing." >&2 exit 2 ;; @@ -210,6 +237,35 @@ if [ -n "$planning_requires" ]; then exit 2 ;; esac p="${p%/}" + # RESOLVE, then compare — do not try to enumerate the ways to spell ".". The first version + # matched a literal table (`. ./ .. ../ / ~`) and `.//`, `./.`, `././`, `.///` sailed straight + # through it: not absolute, no `..`, no glob. `${p%/}` then normalised them and the whole repo + # became the "planning source" — measured on a repo with NO planning docs at all: + # (unconfigured) → rc=1 ok=0/7 NOT ready + # planning_requires: [.//] → rc=0 ok=1/1 "ready — safe to run unattended" + # `.git` did the same in any git repo whatsoever. One resolve-and-compare closes that entire + # family (including symlinks, which is why -P) instead of waiting for the next spelling. + _abs="" + if [ -d "$p" ]; then _abs="$(cd "$p" 2>/dev/null && pwd -P || true)" + elif [ -e "$p" ]; then _abs="$(cd "$(dirname "$p")" 2>/dev/null && pwd -P || true)/$(basename "$p")" + fi + if [ -n "$_abs" ]; then + _root="$(git rev-parse --show-toplevel 2>/dev/null || pwd -P)" + _rootp="$(cd "$_root" 2>/dev/null && pwd -P || printf '%s' "$_root")" + if [ "$_abs" = "$_rootp" ]; then + echo "ERROR: planning_requires entry '$p' resolves to the repository root ($_abs) — that disables the gate rather than configuring it. Point it at the actual planning source (e.g. backlog/tasks)." >&2 + exit 2 + fi + case "$_abs" in + "$_rootp"/.git|"$_rootp"/.git/*) + echo "ERROR: planning_requires entry '$p' points inside .git — it would pass in ANY git repo and proves nothing about planning. Refusing." >&2 + exit 2 ;; + "$_rootp"/*) : ;; # inside the repo, as required + *) + echo "ERROR: planning_requires entry '$p' resolves outside the repository ($_abs) — refusing." >&2 + exit 2 ;; + esac + fi REQ_LIST="${REQ_LIST:-} $p" IFS=',' done diff --git a/plugins/pilot/skills/pilot/templates/pilot.example.yml b/plugins/pilot/skills/pilot/templates/pilot.example.yml index 781cf1b..c00ab80 100644 --- a/plugins/pilot/skills/pilot/templates/pilot.example.yml +++ b/plugins/pilot/skills/pilot/templates/pilot.example.yml @@ -14,12 +14,15 @@ docs_dir: docs/agent # 规划/运行态文档目录 # 起跑门禁就检查这些路径，而不是 docs_dir 下那七个固定文件名。不声明 = 老行为（查七件套）。 # # 判据不变，只是换了查哪里：每条路径都要真有内容（目录 = 底下至少一个够实质的文件）， -# 空目录 / 只有占位符的文件照样判 NOT ready。声明 `.` / 绝对路径 / 通配符会被直接拒绝 -# —— 那是把门禁关掉，不是配置它。 +# 空目录 / 只有占位符的文件照样判 NOT ready。指向仓库根（`.` 的任何拼法）/ `.git` / 绝对路径 / +# 仓库外 / 通配符，都会被直接拒绝 —— 那是把门禁关掉，不是配置它。 +# +# 运行态文档（progress.md 等）仍归 docs_dir，需要一并把关就也列进来。 +# 条目后面可以写 ` # 注释`，解析时会剥掉。 # # planning_requires: # - backlog/tasks # - backlog/docs -# - docs/agent/progress.md # 运行态文档仍归 docs_dir，按需一并列进来 +# - docs/agent/progress.md # PR 的裁决由外部评审服务给出，pilot 只负责盯状态（契约见 reference/review-contract.md） diff --git a/scripts/ci/check-docs-gate.sh b/scripts/ci/check-docs-gate.sh index f40b435..160733d 100755 --- a/scripts/ci/check-docs-gate.sh +++ b/scripts/ci/check-docs-gate.sh @@ -107,6 +107,14 @@ rm -rf "$alt" # unconditionally in any repo holding a single non-empty file. check "planning_requires='.' refused" 2 "$(bash "$GATE" --planning-requires "." --strict >/dev/null 2>&1; echo $?)" check "planning_requires='/' refused" 2 "$(bash "$GATE" --planning-requires "/" --strict >/dev/null 2>&1; echo $?)" +# The repo-root refusal must be by RESOLUTION, not by a table of spellings. A literal table let +# `.//`, `./.`, `././`, `.///` through — none is absolute, contains `..`, or globs — and the gate +# then reported "ready, safe to run unattended" on a repo with no planning documents at all. +# `.git` was the same trick with less typing: it exists in every git repo. +for spelling in ".//" "./." "././" ".///" ".//." ".git" ".git/refs"; do + check "planning_requires='$spelling' refused" 2 \ + "$(bash "$GATE" --planning-requires "$spelling" --strict >/dev/null 2>&1; echo $?)" +done check "planning_requires absolute refused" 2 "$(bash "$GATE" --planning-requires "/etc" --strict >/dev/null 2>&1; echo $?)" # A glob is refused rather than expanded: word-splitting an unquoted list ALSO globs, so `backlog/*` # used to arrive pre-expanded as the paths it matched and the literal check never saw a `*`. @@ -116,6 +124,31 @@ check "planning_requires glob refused" 2 "$(bash "$GATE" --planning-requires # the flag from run() above, THIS is the assertion that goes red instead of the suite silently # testing backlog/ while claiming to test pristine templates. check "--no-config ignores repo declaration" 1 "$(bash "$GATE" --no-config --docs-dir "$tmp" --strict >/dev/null 2>&1; echo $?)" + +# 8. The .pilot.yml parser must read the form this skill's OWN docs use — YAML allows a trailing +# `# comment` on the key line and on every item, and the first parser matched neither. It failed +# SILENTLY: declaration dropped, gate falls back to the seven filenames, repo told "NOT ready" — +# the exact false negative the feature exists to remove, delivered by the feature. +yml="$(mktemp -d)"; mkdir -p "$yml/plan-src" +(cd "$yml" && git init -q .) +{ echo "# 规划源"; echo "这是一份真实内容,用于验证解析器能读到本 skill 文档里那种带行尾注释的写法。"; echo "第二段确保超过阈值。"; } > "$yml/plan-src/tasks.md" + +printf 'base_branch: main\nplanning_requires: # 可选。规划已在别处时声明\n - plan-src # 条目也可以带注释\n' > "$yml/.pilot.yml" +check "config: key+item comments parsed" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" + +printf 'planning_requires: [plan-src] # 行内数组也允许注释\n' > "$yml/.pilot.yml" +check "config: inline array + comment parsed" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" + +# Declared-but-unparseable must be LOUD. A silent fallback here is indistinguishable from the bug. +printf 'planning_requires:\n' > "$yml/.pilot.yml" +check "config: declared but empty aborts" 2 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" +printf 'planning_requires: []\n' > "$yml/.pilot.yml" +check "config: explicit empty list aborts" 2 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" + +# No key at all is NOT an error — it is the documented default (check the seven docs). +printf 'base_branch: main\n' > "$yml/.pilot.yml" +check "config: no key falls back to docs" 1 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" +rm -rf "$yml" rm -rf "$filled" echo From ede9fd78a75fa902637c597998c6061523ef51ff Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Thu, 6 Aug 2026 08:36:32 +0700 Subject: [PATCH 3/5] =?UTF-8?q?fix(pilot):=20=E7=AC=AC=E4=BA=8C=E8=BD=AE?= =?UTF-8?q?=20=E2=80=94=E2=80=94=20=E4=B8=89=E6=9D=A1=20High=20+=20?= =?UTF-8?q?=E4=B8=A4=E6=9D=A1=E6=88=91=E8=87=AA=E5=B7=B1=E5=BC=95=E5=85=A5?= =?UTF-8?q?=E7=9A=84=E5=9B=9E=E5=BD=92?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 三条 High:同一个 resolve 步骤,三件事各自手做,各漏一个口子 1. `.git` 拒绝仍是按名字比字符串,`.GIT` 直接走过去 —— macOS 默认大小写 不敏感,而 bash 的 pwd -P 保留你敲进去的拼法(cd .GIT && pwd -P → …/.GIT), 所以 _abs 永远匹配不上字面量 /.git。实测 .GIT → rc=0 "ready"。 上一个 commit 写着「不要枚举 . 有多少种拼法」,自己却在枚举 .git 的拼法。 2. 守卫按 CWD 解析,而配置按 toplevel 定位 —— 同一份 .pilot.yml 在不同目录 下含义不同:`- .` 在根目录 rc=2、在 sub/ 里 rc=0;反过来合法的 `- plan-src` 在根 rc=0、在 sub/ rc=1,正是这个功能要消灭的那个假 NOT-ready。 3. 非目录分支只解析父目录,指向仓库外的【文件】软链能通过: planfile -> /etc/passwd → rc=0,内容检查直接读穿过去。目录软链是拦住了的, 所以上一条 commit message 里「符号链接一并关掉」只对了一半。 改法:相对【仓库根】解析、解析【条目本身】(realpath)、然后【问 git】它是什么 ——`--resolve-git-dir`(认 .git 目录 + linked worktree 的 .git 文件)加 `--is-inside-git-dir`(认 .git 底下的子路径)。 ## 两条我自己引入的回归 4. 新加的大声 exit 2 会在合法 YAML 上炸:条目正则要求至少一个空格,于是 【零缩进块序列】(合法 YAML,yaml.safe_load 读得出来,也是不少格式化工具的 默认输出)和标量写法都解析不出东西,直接撞上 exit 2 —— 一份能用的配置把 门禁硬停掉,还告诉操作员「格式不对」。这正是被修的那个 bug 的镜像。 正则放成 ^[[:space:]]*- 并加标量分支;exit 2 只留给真正空的声明。 5. 新加的 .git/refs 断言依赖运行环境:在 linked worktree 里(.git 是文件, 也就是 pilot 自己「一 task 一 worktree」教条下的形态)那个路径不存在, 断言无缘无故变红。.GIT 断言有同样的毛病 —— 它只在大小写不敏感的文件系统 上存在,无条件断言会在 Linux runner 上红。前者改用 .git 本身(两种形态都 成立),后者改成有条件断言。 ## 验证 断言 28 → 38 条。而且这次【在两种仓库形态下都跑】:普通 clone 全绿, linked worktree(.git 是文件)也全绿 —— 第一次改完只在主仓库测,拿到 worktree 里立刻红了 2 条,正是评审说的那个形态。 新增覆盖:.git/.GIT/.Git/.GIT/config 全拒;同一配置从根和从子目录同解 (合法的都 rc=0、`- .` 都 rc=2);文件软链和目录软链都拒;零缩进块序列和 标量写法都能解析;真正空的声明仍 exit 2。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- .../pilot/skills/pilot/scripts/check-docs.sh | 71 +++++++++++++++---- scripts/ci/check-docs-gate.sh | 46 +++++++++++- 2 files changed, 103 insertions(+), 14 deletions(-) diff --git a/plugins/pilot/skills/pilot/scripts/check-docs.sh b/plugins/pilot/skills/pilot/scripts/check-docs.sh index e7347da..cfb5eb0 100755 --- a/plugins/pilot/skills/pilot/scripts/check-docs.sh +++ b/plugins/pilot/skills/pilot/scripts/check-docs.sh @@ -125,10 +125,23 @@ if [ -z "$planning_requires" ] && [ "$read_config" = "1" ]; then for (i = 1; i <= n; i++) { t = clean(a[i]); if (t != "") printf "%s,", t } exit } + # Scalar form: `planning_requires: backlog/tasks`. Valid YAML, and the shape someone writes + # first when there is only one path. Must come after the inline-array rule (which matches `[`) + # and before the empty-key rule. + line ~ /^planning_requires:[[:space:]]*[^[:space:][]/ { + v = line; sub(/^planning_requires:[[:space:]]*/, "", v) + t = clean(v); if (t != "") printf "%s,", t + exit + } line ~ /^planning_requires:[[:space:]]*$/ { inlist = 1; next } inlist { - # A block list ends at the first line that is not an indented `- item`. - if (line ~ /^[[:space:]]+-[[:space:]]*[^[:space:]]/) { + # A block list ends at the first line that is not a `- item`. `[[:space:]]*`, NOT `+`: + # a ZERO-INDENT block sequence is valid YAML (yaml.safe_load reads it identically) and is + # what several formatters emit. Requiring indentation made those parse to nothing, which — + # once the "declared but unparseable" abort was added — turned a VALID config into a hard + # stop that told the operator their config was malformed. That is the mirror image of the + # bug being fixed, introduced by the fix for it. + if (line ~ /^[[:space:]]*-[[:space:]]*[^[:space:]]/) { item = line; sub(/^[[:space:]]*-[[:space:]]*/, "", item) t = clean(item); if (t != "") printf "%s,", t next @@ -217,6 +230,10 @@ if [ -n "$planning_requires" ]; then # ok=7/9 from a single declared entry). With globbing off, the entry arrives literal and the # check can refuse it. Restored right after the split. set -f + # Resolved ONCE, outside the loop: every entry is interpreted relative to the repository root, not + # to wherever the operator happens to be standing. See the note at the resolve step below. + _root="$(git rev-parse --show-toplevel 2>/dev/null || pwd -P)" + _rootp="$(cd "$_root" 2>/dev/null && pwd -P || printf '%s' "$_root")" OLDIFS="$IFS"; IFS=',' for p in $planning_requires; do IFS="$OLDIFS" @@ -245,21 +262,46 @@ if [ -n "$planning_requires" ]; then # planning_requires: [.//] → rc=0 ok=1/1 "ready — safe to run unattended" # `.git` did the same in any git repo whatsoever. One resolve-and-compare closes that entire # family (including symlinks, which is why -P) instead of waiting for the next spelling. - _abs="" - if [ -d "$p" ]; then _abs="$(cd "$p" 2>/dev/null && pwd -P || true)" - elif [ -e "$p" ]; then _abs="$(cd "$(dirname "$p")" 2>/dev/null && pwd -P || true)/$(basename "$p")" - fi + # Resolve relative to the REPO ROOT, resolve the ENTRY ITSELF, then ASK GIT what it is. + # Each of those three was previously done by hand, and each hand-rolled version leaked: + # + # 1. `.git` was refused by STRING comparison, so `.GIT` walked straight in — macOS is + # case-insensitive by default and bash's `pwd -P` preserves the spelling you typed + # (`cd .GIT && pwd -P` → `…/.GIT`), so `_abs` never equalled the literal `/.git`. + # Measured: `--planning-requires .GIT` → rc=0 "ready — safe to run unattended". The commit + # that added it said "don't enumerate spellings of `.`" while enumerating spellings of + # `.git`. Now `git rev-parse --is-inside-git-dir` answers it — git knows its own directory. + # 2. Resolution used the CWD while the config is located from the toplevel, so ONE `.pilot.yml` + # meant different things depending on where you stood: `- .` was refused from the root and + # ACCEPTED from `sub/`, and a valid `- plan-src` was rc=0 from the root but rc=1 from `sub/` + # — the same false NOT-ready this feature exists to remove. + # 3. The non-directory branch resolved only the PARENT, so a symlinked FILE pointing out of the + # repo passed: `planfile -> /etc/passwd` → rc=0, and the content test then read through it. + # Directory symlinks WERE caught, which is what "symlinks are handled" was based on — half true. + _abs="$(cd "$_rootp" 2>/dev/null && python3 -c 'import os,sys; print(os.path.realpath(sys.argv[1]))' "$p" 2>/dev/null || true)" if [ -n "$_abs" ]; then - _root="$(git rev-parse --show-toplevel 2>/dev/null || pwd -P)" - _rootp="$(cd "$_root" 2>/dev/null && pwd -P || printf '%s' "$_root")" if [ "$_abs" = "$_rootp" ]; then echo "ERROR: planning_requires entry '$p' resolves to the repository root ($_abs) — that disables the gate rather than configuring it. Point it at the actual planning source (e.g. backlog/tasks)." >&2 exit 2 fi + # Ask git, don't match names. TWO questions, because `.git` has two shapes: + # * `--resolve-git-dir` recognises both a real `.git` DIRECTORY and a `.git` FILE (the + # `gitdir:` pointer a linked worktree gets — pilot's own one-task-one-worktree shape). + # Without it the file form slipped through: realpath does not follow a gitfile, its parent + # is the worktree (not a git dir), so `.git` was accepted and then merely counted as + # "too small" — rc=1 for a size reason, not rc=2 for the real one. + # * `--is-inside-git-dir` catches paths BELOW the git directory (`.GIT/config`). + _gitmeta=0 + git rev-parse --resolve-git-dir "$_abs" >/dev/null 2>&1 && _gitmeta=1 + if [ "$_gitmeta" = "0" ]; then + _chk="$_abs"; [ -d "$_chk" ] || _chk="$(dirname "$_abs")" + [ "$(cd "$_chk" 2>/dev/null && git rev-parse --is-inside-git-dir 2>/dev/null || true)" = "true" ] && _gitmeta=1 + fi + if [ "$_gitmeta" = "1" ]; then + echo "ERROR: planning_requires entry '$p' points inside the git directory — it would pass in ANY git repo and proves nothing about planning. Refusing." >&2 + exit 2 + fi case "$_abs" in - "$_rootp"/.git|"$_rootp"/.git/*) - echo "ERROR: planning_requires entry '$p' points inside .git — it would pass in ANY git repo and proves nothing about planning. Refusing." >&2 - exit 2 ;; "$_rootp"/*) : ;; # inside the repo, as required *) echo "ERROR: planning_requires entry '$p' resolves outside the repository ($_abs) — refusing." >&2 @@ -274,9 +316,12 @@ if [ -n "$planning_requires" ]; then [ -n "${REQ_LIST:-}" ] || { echo "ERROR: planning_requires is set but resolved to an empty list — refusing (an empty list would pass unconditionally)." >&2; exit 2; } for p in $REQ_LIST; do - if [ ! -e "$p" ]; then + # Same repo-root anchoring as the validation above — checking these relative to the CWD is what + # made one config mean two different things depending on which directory the gate ran from. + _pabs="$(cd "$_rootp" 2>/dev/null && python3 -c 'import os,sys; print(os.path.realpath(sys.argv[1]))' "$p" 2>/dev/null || true)" + if [ -z "$_pabs" ] || [ ! -e "$_pabs" ]; then missing="$missing $p" - elif path_has_content "$p"; then + elif path_has_content "$_pabs"; then ok=$((ok + 1)) else empty="$empty $p" diff --git a/scripts/ci/check-docs-gate.sh b/scripts/ci/check-docs-gate.sh index 160733d..2a68707 100755 --- a/scripts/ci/check-docs-gate.sh +++ b/scripts/ci/check-docs-gate.sh @@ -111,10 +111,23 @@ check "planning_requires='/' refused" 2 "$(bash "$GATE" --planning-requires # `.//`, `./.`, `././`, `.///` through — none is absolute, contains `..`, or globs — and the gate # then reported "ready, safe to run unattended" on a repo with no planning documents at all. # `.git` was the same trick with less typing: it exists in every git repo. -for spelling in ".//" "./." "././" ".///" ".//." ".git" ".git/refs"; do +for spelling in ".//" "./." "././" ".///" ".//." ".git"; do check "planning_requires='$spelling' refused" 2 \ "$(bash "$GATE" --planning-requires "$spelling" --strict >/dev/null 2>&1; echo $?)" done +# NOT `.git/refs`, and NOT an unconditional `.GIT`. Both looked like stronger assertions and both +# were environment-dependent — the same class of defect they were asserting against: +# * in a LINKED WORKTREE (`.git` is a file, which is pilot's own one-task-one-worktree shape) +# `.git/refs` does not exist, so the entry is merely MISSING → rc=1 and the assertion goes red +# without anything being wrong; +# * `.GIT` only exists on a case-insensitive filesystem (macOS default), so asserting it +# unconditionally would fail on a case-sensitive CI runner for the same non-reason. +# `.git` itself resolves correctly in both shapes. The case variant is asserted only where the +# filesystem actually makes it reachable. +if [ -e ".GIT" ]; then + check "planning_requires='.GIT' refused (case-insensitive fs)" 2 \ + "$(bash "$GATE" --planning-requires ".GIT" --strict >/dev/null 2>&1; echo $?)" +fi check "planning_requires absolute refused" 2 "$(bash "$GATE" --planning-requires "/etc" --strict >/dev/null 2>&1; echo $?)" # A glob is refused rather than expanded: word-splitting an unquoted list ALSO globs, so `backlog/*` # used to arrive pre-expanded as the paths it matched and the literal check never saw a `*`. @@ -148,6 +161,37 @@ check "config: explicit empty list aborts" 2 "$(cd "$yml" && bash "$GATE_ABS" -- # No key at all is NOT an error — it is the documented default (check the seven docs). printf 'base_branch: main\n' > "$yml/.pilot.yml" check "config: no key falls back to docs" 1 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" + +# 9. Other VALID YAML spellings must parse, not abort. The "declared but unparseable" abort added +# in the previous round only accepted indented block lists, so a zero-indent sequence (valid +# YAML, and what several formatters emit) and the scalar form became a HARD STOP telling the +# operator their working config was malformed — the mirror image of the bug being fixed. +printf 'planning_requires:\n- plan-src\n' > "$yml/.pilot.yml" +check "config: zero-indent block sequence parsed" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" +printf 'planning_requires: plan-src\n' > "$yml/.pilot.yml" +check "config: scalar form parsed" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" + +# 10. One config must mean ONE thing regardless of the directory the gate runs from. Resolving +# entries against the CWD (while the config is found from the toplevel) made `- .` refused at +# the root but ACCEPTED from a subdirectory, and a valid entry rc=0 at the root but rc=1 below. +mkdir -p "$yml/sub" +cp "$yml/plan-src/tasks.md" "$yml/sub/filler.md" +printf 'planning_requires:\n - plan-src\n' > "$yml/.pilot.yml" +check "config: valid entry, from repo root" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" +check "config: valid entry, from subdir" 0 "$(cd "$yml/sub" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" +printf 'planning_requires:\n - .\n' > "$yml/.pilot.yml" +check "config: repo root refused, from root" 2 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" +check "config: repo root refused, from subdir" 2 "$(cd "$yml/sub" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" + +# 11. A symlinked FILE pointing out of the repo must be refused too. Only the parent directory used +# to be resolved, so `planfile -> /etc/passwd` passed the containment check and the content test +# then read straight through it. Directory symlinks were already caught — that half was fine. +ln -sf /etc/passwd "$yml/planfile" +check "config: symlinked file out of repo refused" 2 \ + "$(cd "$yml" && bash "$GATE_ABS" --planning-requires "planfile" --strict >/dev/null 2>&1; echo $?)" +ln -sfn /etc "$yml/outlink" +check "config: symlinked dir out of repo refused" 2 \ + "$(cd "$yml" && bash "$GATE_ABS" --planning-requires "outlink" --strict >/dev/null 2>&1; echo $?)" rm -rf "$yml" rm -rf "$filled" From 36a8d9a389b77fef7e1bff9ef9622343ea40b9e0 Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Thu, 6 Aug 2026 10:20:21 +0700 Subject: [PATCH 4/5] =?UTF-8?q?refactor(pilot):=20=E8=B5=B7=E8=B7=91?= =?UTF-8?q?=E9=97=A8=E7=A6=81=E6=94=B9=E6=88=90=E4=B8=80=E5=8F=A5=E5=A3=B0?= =?UTF-8?q?=E6=98=8E,=E7=A0=8D=E6=8E=89=E6=95=B4=E4=B8=AA=E8=B7=AF?= =?UTF-8?q?=E5=BE=84=E6=A0=A1=E9=AA=8C=E5=99=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 原方案是 planning_requires: [backlog/tasks] —— 让仓库指出真正的规划源, 门禁去【校验那些路径】。两轮评审在这一个能力上找出五个缺陷,而且 【全部在路径校验里】,没有一个在它要解决的那个问题上: - 仓库根的各种拼法 .// ./. ././ 绕过拒绝表 - .GIT 在大小写不敏感的文件系统上绕过 .git 的字符串匹配 - 指向仓库外的【文件】软链(只解析了父目录) - 条目按 CWD 解析、配置按 toplevel 定位 → 同一份配置在不同目录含义不同 - 我自己加的「declared but unparseable」abort 把合法的零缩进 YAML 列表判成错 一个「换个路径去查」的旋钮,必须扛住路径能撒谎的每一种方式,而那是个 比原问题大得多的问题。 改成: planning_source: docs | external external 时门禁【什么都不查】,打印 NOTHING WAS CHECKED 然后放行。 没有判据可以被绕过,因为根本没有判据。 门禁存在的意义是「别在规划不全的时候无人值守开跑」;仓库里的人写下 external,就是显式接过了这个责任 —— 这正是门禁本来要求的东西。代价是 这类仓库的门禁变成不查,但门禁本来只为无人值守而存在,人可以选择不开。 配套要求写进 SKILL.md 和 run.md:汇报时必须照实说「本仓库声明规划在别处、 门禁未核实」,不能说成「规划已验证」——放行是人担保的,不是脚本核实的。 代码:check-docs.sh +304 → +67 行;CI 断言从「九条只为路径能怎么撒谎」 变成六条「每个取值路由到哪、未知值中止、external 必须自报没查」。 额外一条断言:--no-config 必须能忽略这个声明,否则上面每条断言都是空的。 Closes FU-5 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- plugins/pilot/skills/pilot/SKILL.md | 13 +- plugins/pilot/skills/pilot/phases/run.md | 2 +- .../pilot/skills/pilot/scripts/check-docs.sh | 349 ++++-------------- .../skills/pilot/templates/pilot.example.yml | 19 +- scripts/ci/check-docs-gate.sh | 144 ++------ 5 files changed, 115 insertions(+), 412 deletions(-) diff --git a/plugins/pilot/skills/pilot/SKILL.md b/plugins/pilot/skills/pilot/SKILL.md index b1b21dc..3f4ec46 100644 --- a/plugins/pilot/skills/pilot/SKILL.md +++ b/plugins/pilot/skills/pilot/SKILL.md @@ -64,9 +64,7 @@ protect_patterns: [release, hotfix] # 额外保护的分支前缀 remote: origin allow_remote_cleanup: false # 删除远程已合并分支需显式置 true docs_dir: docs/agent # 规划/运行态文档目录 -# 可选。规划已在别处时,声明它的本地路径,起跑门禁就改查这些(不声明 = 老行为:查上面那七个文件名) -planning_requires: - - backlog/tasks +planning_source: docs # docs(默认)=查上面那七个文件 | external=规划在别处,门禁不查(见下) ``` 读取方式：用 Read 工具读该文件，把值作为 flag 传给脚本（脚本本身不解析 YAML，保持简单确定）。文件不存在时用上表默认值，并提示用户运行 `pilot doctor` 生成。 @@ -85,10 +83,11 @@ planning_requires: 3. **规划层齐全度**（`run` 无人值守的硬前提）：`bash /scripts/check-docs.sh --docs-dir --strict`。 报 MISSING/EMPTY 就照实列出并建议 `pilot plan` 补齐——`run` 会在同一道门禁上 fail-closed 拒跑， 在这里先看见比半夜被拦住强。（脚本会识别「文件在但还是原样模板」：占位符没填等于没答。） - **看输出的 `source=` 字段**：`dir=` 表示查的是 docs_dir 下那七个文件；`source=planning_requires(...)` 表示 - 这个仓库在 `.pilot.yml` 里声明了别的规划源，查的是那些路径。**规划已经在 backlog.md / issue tracker / - 别的工具里的仓库,报 NOT ready 时不要建议把规划重抄进七件套**——那违反 plan.md §A.3(已有规划不要重复造)。 - 正确建议是声明 `planning_requires:` 指向真正的规划源。 + **规划已经在 backlog.md / issue tracker / 别的工具里的仓库**：报 NOT ready 时**不要**建议把规划重抄进 + 七件套——那违反 plan.md §A.3(已有规划不要重复造)。正确做法是在 `.pilot.yml` 里写 `planning_source: external`。 + 之后门禁会打印 `source=external — NOTHING WAS CHECKED` 并放行：**它没有检查任何东西**。 + 汇报时必须照实说「本仓库声明规划在别处、门禁未核实」，**不能说成「规划已验证」或「就绪」**—— + 那句放行是人担保的，不是脚本核实的。 4. **集成分支**（`git show-ref refs/heads/`）。**分支不存在时不要一律说「先建一个」**——先分清是哪一种，三种情况的正确答案完全不同： - **`.pilot.yml` 里 `integration_branch` == `base_branch`**（单主干仓库，PR 直接开向主干）→ **这是合法配置，什么都不缺**。不要建议建集成分支。提示：合并走 `git-guard.sh merge-pr --integration --allow-trunk`，并说明 `--allow-trunk` 不是绕过（仍要求分支保护要求审批、PR 已 `APPROVED`、且该分支开启了 stale-dismissal，读不到就 fail-closed）。 - **没有 `.pilot.yml`**（于是 `integration_branch` 落到默认值 `preview`）**且默认分支存在** → **默认值对这个仓库很可能是错的**，别让用户去建一个 `preview` 来迁就默认值。先问：这个仓库是单主干（PR 直接进 `main`）还是双分支流（`preview` 汇总后再进 `main`）？单主干 → 写 `.pilot.yml` 把 `integration_branch` 设成主干名，合并加 `--allow-trunk`；确实要双分支流 → 才建 `preview`。 diff --git a/plugins/pilot/skills/pilot/phases/run.md b/plugins/pilot/skills/pilot/phases/run.md index 090441a..7d8488a 100644 --- a/plugins/pilot/skills/pilot/phases/run.md +++ b/plugins/pilot/skills/pilot/phases/run.md @@ -16,7 +16,7 @@ bash /scripts/check-docs.sh --docs-dir --strict - `rc=0` → 规划层齐全且**已填写**（脚本会识别「还是原样模板」——占位符没填等于没答，比没有更危险），继续 0b。 - **`rc≠0` 一律停下**（`1`=有缺口，`2`=用法/参数错，例如 `PILOT_DOC_MIN_BYTES` 不是正整数）。把脚本输出原样报给用户；`rc=1` 时建议 `pilot plan` 补齐。**不要把非零当成「大概能跑」**。 -- **规划已经在别处的仓库**（backlog.md / issue tracker / 别的工具）：不要为了过门禁去把已有规划重抄一份到七件套里——那正违反 plan.md §A.3。在 `.pilot.yml` 里声明 `planning_requires:` 指向那个规划源的本地路径，脚本会自动读到并改查那些路径（判据不变，仍要求真有内容）。脚本输出的 `source=` 字段会写明这一轮查的是哪一种，照它汇报。 +- **规划已经在别处的仓库**（backlog.md / issue tracker / 别的工具）：不要为了过门禁去把已有规划重抄一份到七件套里——那正违反 plan.md §A.3。在 `.pilot.yml` 里写 `planning_source: external`，门禁会放行并打印 `NOTHING WAS CHECKED`。**这条放行是人担保的，不是脚本核实的**：汇报时照实说「本仓库声明规划在别处、门禁未核实」，绝不能写成「规划已验证」。 **不许带着缺口开跑**，也不许自己动手把文档编出来替用户拍板（违反 SKILL.md 硬约束 7）。 - 用户明确说「有人盯着、先跑起来」→ 才可降级 `--minimal`（只要 roadmap+tasks+progress）， 并在汇报里写明**这是降级运行、缺哪几份文档**。降级是用户的决定，不是你的。 diff --git a/plugins/pilot/skills/pilot/scripts/check-docs.sh b/plugins/pilot/skills/pilot/scripts/check-docs.sh index cfb5eb0..26ca92d 100755 --- a/plugins/pilot/skills/pilot/scripts/check-docs.sh +++ b/plugins/pilot/skills/pilot/scripts/check-docs.sh @@ -1,8 +1,14 @@ #!/usr/bin/env bash # check-docs.sh — gate: is the planning layer complete enough to run UNATTENDED? # -# bash scripts/check-docs.sh [--docs-dir docs/agent] [--strict|--minimal] -# [--planning-requires ] [--no-config] +# bash scripts/check-docs.sh [--docs-dir docs/agent] [--strict|--minimal] [--no-config] +# +# Repos whose planning lives elsewhere (backlog.md, an issue tracker, …) declare it in .pilot.yml: +# +# planning_source: external # docs (default) | external +# +# and this gate then checks NOTHING and says so, instead of reporting a false NOT-ready. See the +# long note at the planning_source block for why that is a declaration rather than a check. # # Unattended delivery means nobody is around to answer "what did you mean here?". # Every gap in the planning layer becomes a guess the model makes alone at 3am, so this @@ -17,19 +23,7 @@ # (`<...>` angle-bracket slots the templates ship with). Shipping a template with the # slots unfilled is worse than no doc — it reads as answered when it isn't. # -# TWO SHAPES OF PLANNING LAYER -# Default: the seven `docs_dir/*.md` files `pilot plan` produces. -# Alternative: a repo that already plans in backlog.md / an issue tracker / anything else -# declares `planning_requires:` in `.pilot.yml` (or passes --planning-requires), and THOSE -# paths are checked instead (`--no-config` ignores the declaration and checks the seven docs -# regardless — for testing the gate itself). Rationale: the fixed seven filenames made the gate unable to -# see an equivalent — often richer — planning source, so `run` fail-closed on repos that -# were in fact ready, while plan.md §A.3 simultaneously says "already planned → do not -# re-create". Both rules could not be obeyed at once. A path is checked with the SAME -# content test as a doc; a directory passes when at least one file under it passes. -# # Exit: 0 = ready, 1 = gaps found (list printed), 2 = usage error. -# ---8<--- end of help set -uo pipefail docs_dir="docs/agent" @@ -63,108 +57,72 @@ if [ "$MIN_BYTES" -eq 0 ]; then exit 2 fi -planning_requires="" -planning_src="flag" read_config=1 - while [ $# -gt 0 ]; do case "$1" in --docs-dir) [ $# -ge 2 ] || { echo "ERROR: --docs-dir needs a path" >&2; exit 2; }; docs_dir="$2"; shift 2 ;; - --planning-requires) - [ $# -ge 2 ] || { echo "ERROR: --planning-requires needs a comma-separated path list" >&2; exit 2; } - planning_requires="$2"; shift 2 ;; - # Test/inspection escape hatch: check ONLY the seven docs under --docs-dir, ignoring whatever - # the repo declares. Needed because reading .pilot.yml would otherwise silently hijack a - # deliberate `--docs-dir ` — scripts/ci/check-docs-gate.sh runs exactly that shape from - # the repo root, and without this flag every one of its assertions would quietly stop testing - # what it says it tests the moment this repo declared planning_requires. - --no-config) read_config=0; shift ;; --strict) mode="strict"; shift ;; --minimal) mode="minimal"; shift ;; - # Print to the sentinel below rather than a hardcoded line range: a range drifts the moment - # anyone adds a line to the header, and it drifts SILENTLY (measured on git-guard.sh: a 4-line - # overrun printed `set -euo pipefail` as if it were help text). - -h|--help) sed -n '2,/^# ---8<--- end of help/p' "$0" | grep -v '^# ---8<---'; exit 0 ;; + # Ignore .pilot.yml entirely. Used by scripts/ci/check-docs-gate.sh, which tests this gate's own + # logic against fixture directories from the repo root — without this, a repo that declares + # `planning_source: external` would make every one of those assertions exit 0 while claiming to + # test something else. + --no-config) read_config=0; shift ;; + -h|--help) sed -n '2,25p' "$0"; exit 0 ;; *) echo "ERROR: unknown arg: $1" >&2; exit 2 ;; esac done -# Read `planning_requires:` from .pilot.yml when the caller did not pass it. +# ---- planning_source: where does this repo's planning actually live? -------------------------- +# +# Default (absent, or `docs`): the seven files under docs_dir, checked as below. +# +# `external`: the repo plans in backlog.md / an issue tracker / anywhere else. The gate then +# verifies NOTHING and says so, loudly, instead of reporting a false NOT-ready. Measured: Brood +# plans in backlog/ (4 milestones, 49 tasks with acceptance criteria, 2 ADRs) and this gate scored +# it 0/7 — while plan.md §A.3 tells you not to re-create planning that already exists. Both rules +# could not be obeyed at once. # -# Same reasoning as git-guard.sh's self-contained protection list: run.md executes as separate -# Bash calls that share no variables, so a caller-threaded flag silently vanishes between steps. -# Here the stakes are the mirror image of git-guard's — forgetting the flag makes this gate MORE -# strict (it falls back to the seven filenames and refuses to run), which is safe but is exactly -# the false "NOT ready" this feature exists to remove. So read the config directly; the flag stays -# available as an override. +# WHY A DECLARATION AND NOT A CHECK. The obvious fix is to let the repo point the gate at its real +# planning source and have the gate verify THAT. I built it: `planning_requires: [backlog/tasks]`, +# with the same content bar. Two review rounds found five defects and every one of them was in the +# path validation, not in the thing it was meant to solve — the repo root spelled `.//`, `./.`, +# `././`; `.GIT` slipping past a `.git` string match on a case-insensitive filesystem; a symlinked +# FILE pointing out of the repo; entries resolved against the CWD while the config was found from +# the toplevel; and my own "declared but unparseable" abort killing a perfectly valid zero-indent +# YAML list. A knob that says "check this path instead" has to be robust against every way a path +# can lie, and that is a much bigger problem than the one being solved. # -# Accepts both YAML shapes: planning_requires: [a, b] and a block list of `- a` lines. -if [ -z "$planning_requires" ] && [ "$read_config" = "1" ]; then +# A declaration has no criterion to subvert, because it has no criterion. The gate exists so nobody +# starts an UNATTENDED run against an incomplete plan; a human writing `external` in the repo's own +# config is taking that responsibility explicitly, which is the same thing the gate was asking for. +planning_source="" +if [ "$read_config" = "1" ]; then _top="$(git rev-parse --show-toplevel 2>/dev/null || echo .)" for _f in "$_top/.pilot.yml" "$_top/.repo-pilot.yml"; do [ -f "$_f" ] || continue - # YAML permits a trailing `# comment` on the key line AND on every item. The first version of - # this parser matched `^planning_requires:[[:space:]]*$`, so the form documented in this very - # skill — `planning_requires: # 可选…` — never matched: `inlist` stayed 0, the declaration was - # dropped SILENTLY, and the gate fell back to the seven filenames. That is precisely the false - # "NOT ready" this feature exists to remove, arriving through the feature itself. Item comments - # were worse: the shipped template's `- docs/agent/progress.md # 运行态…` became the literal - # path `docs/agent/progress.md#运行态…` once whitespace was stripped. - # Strip only WHITESPACE-PRECEDED `#`, which is what YAML calls a comment, so a path that - # legitimately contains `#` survives. - planning_requires="$(awk ' - function clean(s) { - sub(/[[:space:]]+#.*$/, "", s) - gsub(/^[[:space:]]+|[[:space:]]+$/, "", s) - return s - } - { line = $0; sub(/[[:space:]]+#.*$/, "", line) } - line ~ /^planning_requires:[[:space:]]*\[/ { - v = line; sub(/^[^[]*\[/, "", v); sub(/\].*$/, "", v) - n = split(v, a, ",") - for (i = 1; i <= n; i++) { t = clean(a[i]); if (t != "") printf "%s,", t } - exit - } - # Scalar form: `planning_requires: backlog/tasks`. Valid YAML, and the shape someone writes - # first when there is only one path. Must come after the inline-array rule (which matches `[`) - # and before the empty-key rule. - line ~ /^planning_requires:[[:space:]]*[^[:space:][]/ { - v = line; sub(/^planning_requires:[[:space:]]*/, "", v) - t = clean(v); if (t != "") printf "%s,", t - exit - } - line ~ /^planning_requires:[[:space:]]*$/ { inlist = 1; next } - inlist { - # A block list ends at the first line that is not a `- item`. `[[:space:]]*`, NOT `+`: - # a ZERO-INDENT block sequence is valid YAML (yaml.safe_load reads it identically) and is - # what several formatters emit. Requiring indentation made those parse to nothing, which — - # once the "declared but unparseable" abort was added — turned a VALID config into a hard - # stop that told the operator their config was malformed. That is the mirror image of the - # bug being fixed, introduced by the fix for it. - if (line ~ /^[[:space:]]*-[[:space:]]*[^[:space:]]/) { - item = line; sub(/^[[:space:]]*-[[:space:]]*/, "", item) - t = clean(item); if (t != "") printf "%s,", t - next - } - if (line ~ /^[[:space:]]*$/) next - exit - } - ' "$_f" | tr -d "\"'" | sed 's/,$//')" - if [ -n "$planning_requires" ]; then - planning_src="$(basename "$_f")" - elif grep -qE '^planning_requires:' "$_f" 2>/dev/null; then - # The key is THERE but nothing parsed out of it. Never fall back silently: a quiet fallback - # reports "NOT ready, go write the seven docs" to a repo that did declare its planning source, - # which is indistinguishable from the bug this feature fixes. Fail loudly instead. - echo "ERROR: $_f declares 'planning_requires:' but no usable entries parsed out of it." >&2 - echo " Expected either planning_requires: [a, b] or an indented block list of '- path' lines." >&2 - echo " Refusing to silently fall back to the default docs — that would report NOT ready for a repo that DID declare a source." >&2 - exit 2 - fi + planning_source="$(sed -n 's/^planning_source:[[:space:]]*//p' "$_f" | head -1 \ + | sed -e 's/[[:space:]]*#.*$//' -e 's/[[:space:]]*$//' | tr -d "\"'")" break done fi +case "$planning_source" in + ''|docs) : ;; # default: check the seven docs, exactly as before + external) + echo "PILOT_DOCS: mode=$mode source=external (declared in .pilot.yml) — NOTHING WAS CHECKED" + echo " This repo declares its planning lives outside '$docs_dir'. This gate verified nothing:" + echo " it did not look at the planning source and cannot vouch for it." + echo " Report it that way. Do NOT say 'planning verified' or 'ready' — say the repo declares" + echo " its planning is external, and that starting an unattended run asserts it is complete." + exit 0 ;; + *) + echo "ERROR: .pilot.yml has planning_source: '$planning_source' — expected 'docs' or 'external'." >&2 + echo " Refusing rather than guessing: guessing 'docs' would report a false NOT-ready, and" >&2 + echo " guessing 'external' would wave through a repo nobody vouched for." >&2 + exit 2 ;; +esac + # Ordered by the information flow in plan.md: research → acceptance → architecture+spec # → roadmap → tasks → progress. Earlier docs constrain later ones, so report them in order. STRICT_DOCS="research acceptance architecture spec roadmap tasks progress" @@ -187,193 +145,38 @@ real_content_bytes() { | wc -c | tr -d ' ' } -# Does this path hold real content? A file must pass real_content_bytes itself; a directory -# passes when at least ONE file under it does. Directories are the normal case for the -# alternative source (`backlog/tasks/` holds one file per task), and requiring every file to -# pass would fail on the tracker's own scaffolding (empty archive dirs, drafts). -path_has_content() { - local p="$1" f b - if [ -f "$p" ]; then - b="$(real_content_bytes "$p")" - case "$b" in ''|*[!0-9]*) return 1 ;; esac - [ "$b" -ge "$MIN_BYTES" ] && return 0 - return 1 +missing=""; empty=""; ok=0 +for d in $DOCS; do + f="$docs_dir/$d.md" + if [ ! -f "$f" ]; then + missing="$missing $d" + continue fi - if [ -d "$p" ]; then - # -print -quit stops at the first hit, so a huge tracker directory costs one file read. - while IFS= read -r f; do - b="$(real_content_bytes "$f")" - case "$b" in ''|*[!0-9]*) continue ;; esac - [ "$b" -ge "$MIN_BYTES" ] && return 0 - done </dev/null | head -200) -EOF - return 1 + bytes="$(real_content_bytes "$f")" + # Treat an unreadable measurement as EMPTY, never as OK. Same reasoning as the MIN_BYTES + # validation above: if the count is not a plain integer the comparison would error, the + # branch would read false, and the doc would be silently counted as filled in. + case "$bytes" in ''|*[!0-9]*) empty="$empty $d"; continue ;; esac + if [ "$bytes" -lt "$MIN_BYTES" ]; then + empty="$empty $d" + else + ok=$((ok + 1)) fi - return 1 -} - -missing=""; empty=""; ok=0 - -if [ -n "$planning_requires" ]; then - # ---- alternative planning source ----------------------------------------------------------- - # This replaces the seven filenames; it does not relax the content test. Each declared path is - # held to the same MIN_BYTES / no-unfilled-`<...>`-slots bar a doc is. - # - # Refuse paths that would make the gate meaningless by matching the whole repo. `planning_requires: [.]` - # passes trivially in ANY repo with one non-empty file, which is not "configured", it is "off" — - # same reasoning as refusing PILOT_DOC_MIN_BYTES=0 above. The knob is for pointing at a real - # planning source, not for opting out of the gate. - # `set -f` is load-bearing, not tidiness. Word-splitting an unquoted $planning_requires ALSO - # glob-expands it, and that happens BEFORE the loop body — so a `backlog/*` entry silently - # became the nine paths it matched and the glob check below never saw a `*` at all (measured: - # ok=7/9 from a single declared entry). With globbing off, the entry arrives literal and the - # check can refuse it. Restored right after the split. - set -f - # Resolved ONCE, outside the loop: every entry is interpreted relative to the repository root, not - # to wherever the operator happens to be standing. See the note at the resolve step below. - _root="$(git rev-parse --show-toplevel 2>/dev/null || pwd -P)" - _rootp="$(cd "$_root" 2>/dev/null && pwd -P || printf '%s' "$_root")" - OLDIFS="$IFS"; IFS=',' - for p in $planning_requires; do - IFS="$OLDIFS" - # Trim whitespace only. Trailing-slash removal comes AFTER the checks below — stripping first - # turned a bare "/" into "" and it fell out of the loop as "empty list", reporting the wrong - # reason for a refusal that should name what was actually passed. - p="$(printf '%s' "$p" | sed -e 's|^[[:space:]]*||' -e 's|[[:space:]]*$||')" - [ -z "$p" ] && continue - case "$p" in - /*|*..*) - echo "ERROR: planning_requires entry '$p' must be a repo-relative path without '..' — refusing." >&2 - exit 2 ;; - # A glob would be expanded by whoever wrote it (or not at all, becoming a literal path that - # is simply MISSING) — either way the gate would be checking something other than what the - # config appears to say. Require literal paths so the declaration means one fixed thing. - *'*'*|*'?'*|*'['*) - echo "ERROR: planning_requires entry '$p' contains a glob — pass literal paths so the declaration checks exactly what it says." >&2 - exit 2 ;; - esac - p="${p%/}" - # RESOLVE, then compare — do not try to enumerate the ways to spell ".". The first version - # matched a literal table (`. ./ .. ../ / ~`) and `.//`, `./.`, `././`, `.///` sailed straight - # through it: not absolute, no `..`, no glob. `${p%/}` then normalised them and the whole repo - # became the "planning source" — measured on a repo with NO planning docs at all: - # (unconfigured) → rc=1 ok=0/7 NOT ready - # planning_requires: [.//] → rc=0 ok=1/1 "ready — safe to run unattended" - # `.git` did the same in any git repo whatsoever. One resolve-and-compare closes that entire - # family (including symlinks, which is why -P) instead of waiting for the next spelling. - # Resolve relative to the REPO ROOT, resolve the ENTRY ITSELF, then ASK GIT what it is. - # Each of those three was previously done by hand, and each hand-rolled version leaked: - # - # 1. `.git` was refused by STRING comparison, so `.GIT` walked straight in — macOS is - # case-insensitive by default and bash's `pwd -P` preserves the spelling you typed - # (`cd .GIT && pwd -P` → `…/.GIT`), so `_abs` never equalled the literal `/.git`. - # Measured: `--planning-requires .GIT` → rc=0 "ready — safe to run unattended". The commit - # that added it said "don't enumerate spellings of `.`" while enumerating spellings of - # `.git`. Now `git rev-parse --is-inside-git-dir` answers it — git knows its own directory. - # 2. Resolution used the CWD while the config is located from the toplevel, so ONE `.pilot.yml` - # meant different things depending on where you stood: `- .` was refused from the root and - # ACCEPTED from `sub/`, and a valid `- plan-src` was rc=0 from the root but rc=1 from `sub/` - # — the same false NOT-ready this feature exists to remove. - # 3. The non-directory branch resolved only the PARENT, so a symlinked FILE pointing out of the - # repo passed: `planfile -> /etc/passwd` → rc=0, and the content test then read through it. - # Directory symlinks WERE caught, which is what "symlinks are handled" was based on — half true. - _abs="$(cd "$_rootp" 2>/dev/null && python3 -c 'import os,sys; print(os.path.realpath(sys.argv[1]))' "$p" 2>/dev/null || true)" - if [ -n "$_abs" ]; then - if [ "$_abs" = "$_rootp" ]; then - echo "ERROR: planning_requires entry '$p' resolves to the repository root ($_abs) — that disables the gate rather than configuring it. Point it at the actual planning source (e.g. backlog/tasks)." >&2 - exit 2 - fi - # Ask git, don't match names. TWO questions, because `.git` has two shapes: - # * `--resolve-git-dir` recognises both a real `.git` DIRECTORY and a `.git` FILE (the - # `gitdir:` pointer a linked worktree gets — pilot's own one-task-one-worktree shape). - # Without it the file form slipped through: realpath does not follow a gitfile, its parent - # is the worktree (not a git dir), so `.git` was accepted and then merely counted as - # "too small" — rc=1 for a size reason, not rc=2 for the real one. - # * `--is-inside-git-dir` catches paths BELOW the git directory (`.GIT/config`). - _gitmeta=0 - git rev-parse --resolve-git-dir "$_abs" >/dev/null 2>&1 && _gitmeta=1 - if [ "$_gitmeta" = "0" ]; then - _chk="$_abs"; [ -d "$_chk" ] || _chk="$(dirname "$_abs")" - [ "$(cd "$_chk" 2>/dev/null && git rev-parse --is-inside-git-dir 2>/dev/null || true)" = "true" ] && _gitmeta=1 - fi - if [ "$_gitmeta" = "1" ]; then - echo "ERROR: planning_requires entry '$p' points inside the git directory — it would pass in ANY git repo and proves nothing about planning. Refusing." >&2 - exit 2 - fi - case "$_abs" in - "$_rootp"/*) : ;; # inside the repo, as required - *) - echo "ERROR: planning_requires entry '$p' resolves outside the repository ($_abs) — refusing." >&2 - exit 2 ;; - esac - fi - REQ_LIST="${REQ_LIST:-} $p" - IFS=',' - done - IFS="$OLDIFS" - set +f - [ -n "${REQ_LIST:-}" ] || { echo "ERROR: planning_requires is set but resolved to an empty list — refusing (an empty list would pass unconditionally)." >&2; exit 2; } +done - for p in $REQ_LIST; do - # Same repo-root anchoring as the validation above — checking these relative to the CWD is what - # made one config mean two different things depending on which directory the gate ran from. - _pabs="$(cd "$_rootp" 2>/dev/null && python3 -c 'import os,sys; print(os.path.realpath(sys.argv[1]))' "$p" 2>/dev/null || true)" - if [ -z "$_pabs" ] || [ ! -e "$_pabs" ]; then - missing="$missing $p" - elif path_has_content "$_pabs"; then - ok=$((ok + 1)) - else - empty="$empty $p" - fi - done - total=$(echo $REQ_LIST | wc -w | tr -d ' ') - echo "PILOT_DOCS: mode=$mode source=planning_requires($planning_src) min_bytes=$MIN_BYTES ok=$ok/$total" - echo " declared planning source:$REQ_LIST" -else - for d in $DOCS; do - f="$docs_dir/$d.md" - if [ ! -f "$f" ]; then - missing="$missing $d" - continue - fi - bytes="$(real_content_bytes "$f")" - # Treat an unreadable measurement as EMPTY, never as OK. Same reasoning as the MIN_BYTES - # validation above: if the count is not a plain integer the comparison would error, the - # branch would read false, and the doc would be silently counted as filled in. - case "$bytes" in ''|*[!0-9]*) empty="$empty $d"; continue ;; esac - if [ "$bytes" -lt "$MIN_BYTES" ]; then - empty="$empty $d" - else - ok=$((ok + 1)) - fi - done - total=$(echo $DOCS | wc -w | tr -d ' ') - # Echo the effective threshold on EVERY outcome, including success. PILOT_DOC_MIN_BYTES can - # weaken the gate from outside the repo; if the passing line never showed it, a loosened gate - # would leave no trace in the run's report. - echo "PILOT_DOCS: mode=$mode dir=$docs_dir min_bytes=$MIN_BYTES ok=$ok/$total" -fi +total=$(echo $DOCS | wc -w | tr -d ' ') +# Echo the effective threshold on EVERY outcome, including success. PILOT_DOC_MIN_BYTES can +# weaken the gate from outside the repo; if the passing line never showed it, a loosened gate +# would leave no trace in the run's report. +echo "PILOT_DOCS: mode=$mode dir=$docs_dir min_bytes=$MIN_BYTES ok=$ok/$total" if [ -z "$missing" ] && [ -z "$empty" ]; then echo "PILOT_DOCS: ready — planning layer complete, safe to run unattended." exit 0 fi -# In planning_requires mode the entries ARE paths already — prefixing them with $docs_dir would -# print a path that does not exist and send whoever reads it to the wrong place. -if [ -n "$planning_requires" ]; then - [ -n "$missing" ] && { echo " MISSING (path absent):"; for d in $missing; do echo " - $d"; done; } - [ -n "$empty" ] && { echo " EMPTY (no file with ${MIN_BYTES}B+ of real content under it):"; for d in $empty; do echo " - $d"; done; } - echo "PILOT_DOCS: NOT ready — the planning source declared in .pilot.yml is incomplete." - echo " Fix the source itself, or correct \`planning_requires:\` if it points at the wrong paths." - echo " (do NOT switch back to the seven docs just to get past this — that re-creates planning that already exists elsewhere; see plan.md §A.3)" - exit 1 -fi - [ -n "$missing" ] && { echo " MISSING (file absent):"; for d in $missing; do echo " - $docs_dir/$d.md"; done; } [ -n "$empty" ] && { echo " EMPTY (still the template / under ${MIN_BYTES}B of real content):"; for d in $empty; do echo " - $docs_dir/$d.md"; done; } echo "PILOT_DOCS: NOT ready — run \`pilot plan\` to fill these in before an unattended run." echo " (a human-supervised run can proceed with --minimal: roadmap + tasks + progress)" -echo " (already plan in backlog.md / an issue tracker? declare \`planning_requires:\` in .pilot.yml so this gate checks THAT instead — see the header of this script)" exit 1 diff --git a/plugins/pilot/skills/pilot/templates/pilot.example.yml b/plugins/pilot/skills/pilot/templates/pilot.example.yml index c00ab80..fb68a81 100644 --- a/plugins/pilot/skills/pilot/templates/pilot.example.yml +++ b/plugins/pilot/skills/pilot/templates/pilot.example.yml @@ -10,19 +10,10 @@ remote: origin allow_remote_cleanup: false # 删除远程已合并分支需显式置 true（无人值守默认不删远程） docs_dir: docs/agent # 规划/运行态文档目录 -# 规划已经在别处（backlog.md / issue tracker / 任何工具）时，在这里声明它的本地产物路径， -# 起跑门禁就检查这些路径，而不是 docs_dir 下那七个固定文件名。不声明 = 老行为（查七件套）。 -# -# 判据不变，只是换了查哪里：每条路径都要真有内容（目录 = 底下至少一个够实质的文件）， -# 空目录 / 只有占位符的文件照样判 NOT ready。指向仓库根（`.` 的任何拼法）/ `.git` / 绝对路径 / -# 仓库外 / 通配符，都会被直接拒绝 —— 那是把门禁关掉，不是配置它。 -# -# 运行态文档（progress.md 等）仍归 docs_dir，需要一并把关就也列进来。 -# 条目后面可以写 ` # 注释`，解析时会剥掉。 -# -# planning_requires: -# - backlog/tasks -# - backlog/docs -# - docs/agent/progress.md +# 规划在哪里。docs（默认）= 起跑门禁检查 docs_dir 下那七个文件； +# external = 规划在别处（backlog.md / issue tracker / 任何工具），门禁【不做任何检查】直接放行。 +# external 是一句人做的担保，不是脚本核实的结论 —— 门禁会明确打印「NOTHING WAS CHECKED」， +# 汇报时必须照实转述，不能说成「规划已验证」。 +planning_source: docs # PR 的裁决由外部评审服务给出，pilot 只负责盯状态（契约见 reference/review-contract.md） diff --git a/scripts/ci/check-docs-gate.sh b/scripts/ci/check-docs-gate.sh index 2a68707..66d33c3 100755 --- a/scripts/ci/check-docs-gate.sh +++ b/scripts/ci/check-docs-gate.sh @@ -12,9 +12,6 @@ set -uo pipefail GATE="plugins/pilot/skills/pilot/scripts/check-docs.sh" -# Absolute form: assertion 5 must `cd` into a fixture (planning_requires entries are repo-relative -# by design), and a relative $GATE stops resolving the moment we leave the repo root. -GATE_ABS="$PWD/$GATE" TEMPLATES="plugins/pilot/skills/pilot/templates" fails=0 @@ -41,10 +38,10 @@ check() { # check fi } -# --no-config on EVERY call: these assertions are about the gate's own logic against a fixture -# directory, and this script runs from the repo root. Without it, the gate would read this repo's -# .pilot.yml, see `planning_requires:`, and check backlog/ instead of $tmp — every assertion below -# would keep printing "ok" while testing something else entirely. Assertion 7 pins that down. +GATE_ABS="$PWD/$GATE" +# --no-config on every fixture call: these assertions are about the gate's own logic, and this +# script runs from the repo root. Without it, a repo declaring `planning_source: external` would +# make every assertion below exit 0 while still printing "ok". run() { bash "$GATE" --no-config --docs-dir "$tmp" "$@" >/dev/null 2>&1; echo $?; } echo "check-docs.sh fail-closed assertions:" @@ -81,119 +78,32 @@ for d in research acceptance architecture spec roadmap tasks progress; do } > "$filled/$d.md" done check "filled docs accepted" 0 "$(bash "$GATE" --no-config --docs-dir "$filled" --strict >/dev/null 2>&1; echo $?)" - rm -rf "$filled" -# 5. An alternative planning source (`planning_requires:`) must be held to the SAME content bar. -# It replaces WHICH paths are checked, never WHETHER they have to be real — a knob that let a -# repo declare its way past the gate would be the fail-open this whole script exists to catch. -# Run from inside the fixture (entries must be repo-relative), so the gate needs an ABS path. -alt="$(mktemp -d)"; mkdir -p "$alt/plan-src" "$alt/blank-src" -: > "$alt/blank-src/placeholder.md" -{ - echo "# 规划源" - echo "本文件包含足够的实质内容,用于验证门禁在替代规划源下能够正常放行,而不是一律拒绝。" - echo "第二段补充说明,确保真实内容超过最小字节阈值。" -} > "$alt/plan-src/tasks.md" -check "planning_requires: dir with real content accepted" 0 \ - "$(cd "$alt" && bash "$GATE_ABS" --planning-requires "plan-src" --strict >/dev/null 2>&1; echo $?)" -check "planning_requires: dir of blank files rejected" 1 \ - "$(cd "$alt" && bash "$GATE_ABS" --planning-requires "blank-src" --strict >/dev/null 2>&1; echo $?)" -check "planning_requires: absent path rejected" 1 \ - "$(cd "$alt" && bash "$GATE_ABS" --planning-requires "plan-src,nope" --strict >/dev/null 2>&1; echo $?)" -rm -rf "$alt" - -# 6. The knob must not be usable to switch the gate OFF. Each of these would otherwise pass -# unconditionally in any repo holding a single non-empty file. -check "planning_requires='.' refused" 2 "$(bash "$GATE" --planning-requires "." --strict >/dev/null 2>&1; echo $?)" -check "planning_requires='/' refused" 2 "$(bash "$GATE" --planning-requires "/" --strict >/dev/null 2>&1; echo $?)" -# The repo-root refusal must be by RESOLUTION, not by a table of spellings. A literal table let -# `.//`, `./.`, `././`, `.///` through — none is absolute, contains `..`, or globs — and the gate -# then reported "ready, safe to run unattended" on a repo with no planning documents at all. -# `.git` was the same trick with less typing: it exists in every git repo. -for spelling in ".//" "./." "././" ".///" ".//." ".git"; do - check "planning_requires='$spelling' refused" 2 \ - "$(bash "$GATE" --planning-requires "$spelling" --strict >/dev/null 2>&1; echo $?)" -done -# NOT `.git/refs`, and NOT an unconditional `.GIT`. Both looked like stronger assertions and both -# were environment-dependent — the same class of defect they were asserting against: -# * in a LINKED WORKTREE (`.git` is a file, which is pilot's own one-task-one-worktree shape) -# `.git/refs` does not exist, so the entry is merely MISSING → rc=1 and the assertion goes red -# without anything being wrong; -# * `.GIT` only exists on a case-insensitive filesystem (macOS default), so asserting it -# unconditionally would fail on a case-sensitive CI runner for the same non-reason. -# `.git` itself resolves correctly in both shapes. The case variant is asserted only where the -# filesystem actually makes it reachable. -if [ -e ".GIT" ]; then - check "planning_requires='.GIT' refused (case-insensitive fs)" 2 \ - "$(bash "$GATE" --planning-requires ".GIT" --strict >/dev/null 2>&1; echo $?)" +# 5. `planning_source:` — a DECLARATION, so the only things to assert are that each value routes +# where it says, that an unrecognised value aborts rather than guessing, and that the external +# path is unmistakably labelled as "not checked". There is no criterion here to subvert, which +# is the entire point of it being a declaration; the previous design (a configurable path to +# verify) needed nine assertions just for the ways a path can lie. +ps="$(mktemp -d)"; (cd "$ps" && git init -q .) +psrun() { printf '%s\n' "$1" > "$ps/.pilot.yml"; (cd "$ps" && bash "$GATE_ABS" --strict >/dev/null 2>&1); echo $?; } +check "planning_source: external → pass" 0 "$(psrun 'planning_source: external')" +check "planning_source: docs → checks docs" 1 "$(psrun 'planning_source: docs')" +check "planning_source: unknown → abort" 2 "$(psrun 'planning_source: backlog')" +check "no planning_source → checks docs" 1 "$(psrun 'base_branch: main')" +check "trailing comment tolerated" 0 "$(psrun 'planning_source: external # 规划在 backlog/')" +# The external path MUST say it checked nothing — a "ready" here would be a lie the run report +# would then repeat. +printf 'planning_source: external\n' > "$ps/.pilot.yml" +if (cd "$ps" && bash "$GATE_ABS" --strict 2>&1) | grep -q "NOTHING WAS CHECKED"; then + echo " ok external run states it verified nothing" +else + echo " FAIL external run does not say it verified nothing" >&2; fails=$((fails + 1)) fi -check "planning_requires absolute refused" 2 "$(bash "$GATE" --planning-requires "/etc" --strict >/dev/null 2>&1; echo $?)" -# A glob is refused rather than expanded: word-splitting an unquoted list ALSO globs, so `backlog/*` -# used to arrive pre-expanded as the paths it matched and the literal check never saw a `*`. -check "planning_requires glob refused" 2 "$(bash "$GATE" --planning-requires "backlog/*" --strict >/dev/null 2>&1; echo $?)" - -# 7. --no-config must actually isolate the fixture from this repo's declaration. If someone drops -# the flag from run() above, THIS is the assertion that goes red instead of the suite silently -# testing backlog/ while claiming to test pristine templates. -check "--no-config ignores repo declaration" 1 "$(bash "$GATE" --no-config --docs-dir "$tmp" --strict >/dev/null 2>&1; echo $?)" - -# 8. The .pilot.yml parser must read the form this skill's OWN docs use — YAML allows a trailing -# `# comment` on the key line and on every item, and the first parser matched neither. It failed -# SILENTLY: declaration dropped, gate falls back to the seven filenames, repo told "NOT ready" — -# the exact false negative the feature exists to remove, delivered by the feature. -yml="$(mktemp -d)"; mkdir -p "$yml/plan-src" -(cd "$yml" && git init -q .) -{ echo "# 规划源"; echo "这是一份真实内容,用于验证解析器能读到本 skill 文档里那种带行尾注释的写法。"; echo "第二段确保超过阈值。"; } > "$yml/plan-src/tasks.md" - -printf 'base_branch: main\nplanning_requires: # 可选。规划已在别处时声明\n - plan-src # 条目也可以带注释\n' > "$yml/.pilot.yml" -check "config: key+item comments parsed" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" - -printf 'planning_requires: [plan-src] # 行内数组也允许注释\n' > "$yml/.pilot.yml" -check "config: inline array + comment parsed" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" - -# Declared-but-unparseable must be LOUD. A silent fallback here is indistinguishable from the bug. -printf 'planning_requires:\n' > "$yml/.pilot.yml" -check "config: declared but empty aborts" 2 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" -printf 'planning_requires: []\n' > "$yml/.pilot.yml" -check "config: explicit empty list aborts" 2 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" - -# No key at all is NOT an error — it is the documented default (check the seven docs). -printf 'base_branch: main\n' > "$yml/.pilot.yml" -check "config: no key falls back to docs" 1 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" - -# 9. Other VALID YAML spellings must parse, not abort. The "declared but unparseable" abort added -# in the previous round only accepted indented block lists, so a zero-indent sequence (valid -# YAML, and what several formatters emit) and the scalar form became a HARD STOP telling the -# operator their working config was malformed — the mirror image of the bug being fixed. -printf 'planning_requires:\n- plan-src\n' > "$yml/.pilot.yml" -check "config: zero-indent block sequence parsed" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" -printf 'planning_requires: plan-src\n' > "$yml/.pilot.yml" -check "config: scalar form parsed" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" - -# 10. One config must mean ONE thing regardless of the directory the gate runs from. Resolving -# entries against the CWD (while the config is found from the toplevel) made `- .` refused at -# the root but ACCEPTED from a subdirectory, and a valid entry rc=0 at the root but rc=1 below. -mkdir -p "$yml/sub" -cp "$yml/plan-src/tasks.md" "$yml/sub/filler.md" -printf 'planning_requires:\n - plan-src\n' > "$yml/.pilot.yml" -check "config: valid entry, from repo root" 0 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" -check "config: valid entry, from subdir" 0 "$(cd "$yml/sub" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" -printf 'planning_requires:\n - .\n' > "$yml/.pilot.yml" -check "config: repo root refused, from root" 2 "$(cd "$yml" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" -check "config: repo root refused, from subdir" 2 "$(cd "$yml/sub" && bash "$GATE_ABS" --strict >/dev/null 2>&1; echo $?)" - -# 11. A symlinked FILE pointing out of the repo must be refused too. Only the parent directory used -# to be resolved, so `planfile -> /etc/passwd` passed the containment check and the content test -# then read straight through it. Directory symlinks were already caught — that half was fine. -ln -sf /etc/passwd "$yml/planfile" -check "config: symlinked file out of repo refused" 2 \ - "$(cd "$yml" && bash "$GATE_ABS" --planning-requires "planfile" --strict >/dev/null 2>&1; echo $?)" -ln -sfn /etc "$yml/outlink" -check "config: symlinked dir out of repo refused" 2 \ - "$(cd "$yml" && bash "$GATE_ABS" --planning-requires "outlink" --strict >/dev/null 2>&1; echo $?)" -rm -rf "$yml" -rm -rf "$filled" +# And --no-config must ignore the declaration, or every assertion above this line is vacuous. +check "--no-config ignores declaration" 1 \ + "$(cd "$ps" && bash "$GATE_ABS" --strict --no-config >/dev/null 2>&1; echo $?)" +rm -rf "$ps" echo if [ "$fails" -ne 0 ]; then From 09503fcba103714780918bda811d0765cb04fd9e Mon Sep 17 00:00:00 2001 From: jhfnetboy Date: Thu, 6 Aug 2026 10:48:41 +0700 Subject: [PATCH 5/5] =?UTF-8?q?fix(pilot):=20=E7=AC=AC=E5=9B=9B=E8=BD=AE?= =?UTF-8?q?=20=E2=80=94=E2=80=94=20CI=20=E6=96=AD=E8=A8=80=E7=9A=84=20SIGP?= =?UTF-8?q?IPE=20=E6=8A=96=E5=8A=A8=20+=20=E5=A3=B0=E6=98=8E=E8=A7=A3?= =?UTF-8?q?=E6=9E=90=E5=99=A8=20fail-open?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 1. [High] `| grep -q` 让 docs-gate 三分之一概率随机变红 grep -q 匹配到第一行就退出,而 gate 还要再打印 4 行 → SIGPIPE → 141; set -o pipefail 把 141 提成整条管道的状态,if 走 else —— banner 明明 正确打印了,断言却判失败。而它报的是「the planning-docs gate is not fail-closed」,恰恰是最容易让人以为门禁真坏了的那句话。 自测复现:修复前 15 次里红 5 次(33%),与评审的采样同量级。 改成捕获到变量 + case 匹配,不用管道。修复后 30 次全绿。 这个 bug 我今天刚在 safe-cleanup.sh 里修过 —— 那里为它写了 list_has 辅助函数,然后我在同一批 PR 的 CI 脚本里又踩了一遍。注释里记了这件事。 ## 2. [Medium] 声明解析器把「根本不是那种 YAML」当成 external 声明去掉了判据,但没去掉读它的那 6 行 sed —— 而那 6 行有着和被删掉的 校验器同样的 fail-OPEN 形状,只是小一号,落点更糟:以前是路径判断出错, 现在是整个门禁被无声关掉。 planning_source:external → YAML 里这是个纯标量字符串,根本没有 key planning_source:external → yaml.safe_load 报 ScannerError planning_source: ex"ter"nal → 值就是 ex"ter"nal,该走「拒绝而不是猜」 三种全被读成 external。改: - 要求一个【字面空格】s/^planning_source: \{1,\}//p。不能用 [[:space:]], 它包含 TAB,而 YAML 不接受 —— 匹配它等于接受一份任何 YAML 解析器都 读不了的文件。前两种因此不匹配,落回查 docs(fail-closed)。 - 只脱【成对】引号,不再 tr -d。第三种因此保持 ex"ter"nal,被 case 的 兜底分支拒绝(exit 2),而不是被抹成 external。 七种写法逐一实测,并与 python yaml.safe_load 的判断对照一致。 三个畸形用例 + 成对引号用例全部进 CI 断言。 Claude-Session: https://claude.ai/code/session_01CmAW1q62bBtjT99inZeyLk --- .../pilot/skills/pilot/scripts/check-docs.sh | 20 +++++++++++-- scripts/ci/check-docs-gate.sh | 28 +++++++++++++++---- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/plugins/pilot/skills/pilot/scripts/check-docs.sh b/plugins/pilot/skills/pilot/scripts/check-docs.sh index 26ca92d..b57ba71 100755 --- a/plugins/pilot/skills/pilot/scripts/check-docs.sh +++ b/plugins/pilot/skills/pilot/scripts/check-docs.sh @@ -101,8 +101,24 @@ if [ "$read_config" = "1" ]; then _top="$(git rev-parse --show-toplevel 2>/dev/null || echo .)" for _f in "$_top/.pilot.yml" "$_top/.repo-pilot.yml"; do [ -f "$_f" ] || continue - planning_source="$(sed -n 's/^planning_source:[[:space:]]*//p' "$_f" | head -1 \ - | sed -e 's/[[:space:]]*#.*$//' -e 's/[[:space:]]*$//' | tr -d "\"'")" + # Require a REAL separating space, and strip only MATCHED quotes. + # + # The declaration removes the criterion, but not the 6 lines that read it — and those lines had + # the same fail-OPEN shape as the validator they replaced, one size smaller. All three of these + # were read as `external` (i.e. gate off), and none of them is what it looks like: + # planning_source:external → to YAML this is a plain SCALAR STRING; there is no key here + # planning_source:external → yaml.safe_load raises ScannerError + # planning_source: ex"ter"nal → the value IS `ex"ter"nal`; it must hit the `*)` refuse arm, + # but `tr -d "\"'"` mangled it into `external` + # `[[:space:]]\{1,\}` makes the first two miss the pattern entirely, so they fall back to + # checking the docs (fail-CLOSED); the paired-quote strip leaves the third as `ex"ter"nal`, + # which the case below refuses instead of guessing. + # A literal SPACE, not `[[:space:]]` — that class includes TAB, and YAML does not: a tab after + # the colon is a ScannerError, not a value. Matching it would have accepted a file no YAML + # parser will read. + planning_source="$(sed -n 's/^planning_source: \{1,\}//p' "$_f" | head -1 \ + | sed -e 's/[[:space:]]*#.*$//' -e 's/[[:space:]]*$//' \ + -e 's/^"\(.*\)"$/\1/' -e "s/^'\(.*\)'$/\1/")" break done fi diff --git a/scripts/ci/check-docs-gate.sh b/scripts/ci/check-docs-gate.sh index 66d33c3..aa9d8d2 100755 --- a/scripts/ci/check-docs-gate.sh +++ b/scripts/ci/check-docs-gate.sh @@ -92,14 +92,32 @@ check "planning_source: docs → checks docs" 1 "$(psrun 'planning_source: docs' check "planning_source: unknown → abort" 2 "$(psrun 'planning_source: backlog')" check "no planning_source → checks docs" 1 "$(psrun 'base_branch: main')" check "trailing comment tolerated" 0 "$(psrun 'planning_source: external # 规划在 backlog/')" +check "quoted value tolerated" 0 "$(psrun 'planning_source: "external"')" +# Malformed YAML must fall CLOSED, never open. The declaration removes the criterion but not the +# sed that reads it, and that sed had the same fail-open shape as the validator it replaced: +# each of these was read as `external` — i.e. gate off — and none is what it looks like. +# no space → to YAML this is a plain scalar string; there is no key at all +# tab → yaml.safe_load raises ScannerError (YAML does not accept a tab there) +# ex"ter"nal→ the value really is ex"ter"nal and must be REFUSED, not mangled into external +check "no space after colon → checks docs" 1 "$(psrun 'planning_source:external')" +check "tab after colon → checks docs" 1 "$(printf 'planning_source:\texternal\n' > "$ps/.pilot.yml"; (cd "$ps" && bash "$GATE_ABS" --strict >/dev/null 2>&1); echo $?)" +check "inner quotes → abort, not guess" 2 "$(psrun 'planning_source: ex"ter"nal')" # The external path MUST say it checked nothing — a "ready" here would be a lie the run report # would then repeat. +# Capture to a variable and match in bash — NOT `… | grep -q`. `grep -q` exits at the first +# match while the gate still has 4 lines to print, so the gate takes SIGPIPE and exits 141; +# `set -o pipefail` (line 12) promotes that to the pipeline's status and the `if` takes the +# ELSE branch — the banner printed correctly and the assertion failed anyway. Measured on the +# previous commit: 5 red out of 15 full runs, each claiming "the planning-docs gate is not +# fail-closed" on a `docs-gate` job with no continue-on-error. This is the same SIGPIPE/pipefail +# trap safe-cleanup.sh has a helper (`list_has`) to avoid; I wrote that helper and then walked +# into it again here, in the same batch of PRs. printf 'planning_source: external\n' > "$ps/.pilot.yml" -if (cd "$ps" && bash "$GATE_ABS" --strict 2>&1) | grep -q "NOTHING WAS CHECKED"; then - echo " ok external run states it verified nothing" -else - echo " FAIL external run does not say it verified nothing" >&2; fails=$((fails + 1)) -fi +_psout="$(cd "$ps" && bash "$GATE_ABS" --strict 2>&1)" +case "$_psout" in + *"NOTHING WAS CHECKED"*) echo " ok external run states it verified nothing" ;; + *) echo " FAIL external run does not say it verified nothing" >&2; fails=$((fails + 1)) ;; +esac # And --no-config must ignore the declaration, or every assertion above this line is vacuous. check "--no-config ignores declaration" 1 \ "$(cd "$ps" && bash "$GATE_ABS" --strict --no-config >/dev/null 2>&1; echo $?)"