From 07795619a98ce5223742e56ac545c769848d149d Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:19:11 +0800 Subject: [PATCH 01/11] =?UTF-8?q?fix:=20migrate-idd-config=20=E5=8F=AA?= =?UTF-8?q?=E8=82=AF=E8=99=95=E7=90=86=E4=B8=80=E8=88=AC=E6=AA=94=E6=A1=88?= =?UTF-8?q?=EF=BC=8C=E4=B8=94=E7=94=A8=20ln=20=E5=8F=96=E5=BE=97=20mv=20?= =?UTF-8?q?=E6=B2=92=E6=9C=89=E7=9A=84=20no-clobber?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 跨模型（Codex）在 post-merge audit 指出四個 CRITICAL：`.idd` 可以是 symlink、breadcrumb 的 `>` 會跟隨 symlink、目的檔只用 `[ -f ]` 檢查、 `find` 只比對檔名。#319 修掉了路徑切割那個（`-print0`），這四個沒修。 共同根因是同一句沒被說出口的假設：**這些路徑都是一般檔案與一般目錄**。 腳本沒有檢查它，於是由 shell 自己「跟隨 symlink」的預設替使用者的資料 做決定。`--apply` 預設走遍整個 ~/Developer，所以「我 clone 下來的 repo」 就在威脅模型內，而上面每一種形狀都能被 commit 進 repo 裡。 - `.idd` 是 symlink → `mkdir -p` 靜默接受並回報成功，config 被搬出樹外 - 目的檔是目錄 → `mv` 把檔案搬**進去**，變成 `local.json/issue-…json`， 然後印「✓ migrated」 - 目的檔是懸空 symlink → `-e` 為 false，沒有任何警告 - 目的檔是指向樹外一般檔的 symlink → `-f` 跟隨它，稽核印出「both present, current wins」——一句在「current 其實不在這個 repo 裡」時為假的話 - breadcrumb 路徑是懸空 symlink → `-e` 看不見，`>` 直接**建立**目標 - `-name` 會撈到 doc sample / fixture，在別人的樹裡憑空造出 `.idd` `-f`、`-d`、`-e` 都會跟隨 symlink，所以順序不是風格問題：`-L` 必須先測， 否則一個連結是由它指向的東西受審。 `mv` 沒有原子的 no-clobber——檢查與 rename 是兩個 syscall，中間有窗口。 `ln` 在目的存在時原子失敗，所以 hardlink-then-unlink 給得出 `mv` 給不出的 保證，且中途被打斷時留下兩個 link 指向同一 inode（config 兩邊都讀得到， 不會兩邊都沒有）。`mv -n` 是另一個候選，但 BSD 上它的 exit status 分不出 「已搬移」與「因目的已存在而跳過」。 測試：hostile tree 自成一個 scan root、自己一次 `--apply`（拒絕算 failure， 混進 happy path 會讓「apply exits 0」變成空話）。7 種形狀 + 1 條回歸鎖 （`find -type f` 用 lstat，所以 legacy 路徑是 symlink 時本來就不會被跟隨； 釘住它是因為這個保護藏在一個 flag 裡、看不見，將來有人改成 `find -L` 會安靜地開始跟隨）。30 assertions，全 suite 47/47。 acid（逐條還原、確認 suite 會紅）：7 個機制裡 6 個單獨可轉紅。`ln` 在預設 組態下被前面兩個守衛遮住，把它們一起關掉、只變動 ln↔mv，紅的數量從 4 變 6——它是活的縱深防禦、不是裝飾。**mkdir 之後的 .idd 複查（M2）沒有測試 重量**：它只窄化 TOCTOU 窗口，單一 process 的測試碰不到，如實記在這裡。 --- .../scripts/migrate-idd-config.sh | 77 +++++++++++-- .../scripts/tests/migrate-idd-config/test.sh | 104 ++++++++++++++++++ 2 files changed, 171 insertions(+), 10 deletions(-) diff --git a/plugins/issue-driven-dev/scripts/migrate-idd-config.sh b/plugins/issue-driven-dev/scripts/migrate-idd-config.sh index 978123f..9ce950f 100755 --- a/plugins/issue-driven-dev/scripts/migrate-idd-config.sh +++ b/plugins/issue-driven-dev/scripts/migrate-idd-config.sh @@ -66,6 +66,35 @@ for root in "${ROOTS[@]}"; do current="$dir/.idd/local.json" repo=$(dirname "$dir") + # ── Everything below assumes ordinary files in ordinary directories. Say so + # and check it, rather than letting the shell's own follow-the-symlink + # defaults decide what happens to somebody's data. The scan root defaults to + # ALL of ~/Developer, so "a repo I cloned" is inside the threat model, and + # every one of these shapes can be committed into a repo. + # + # Test order matters: -f, -d and -e all FOLLOW symlinks, so a link has to be + # excluded with -L first or it is judged by what it points at. + + # The destination's parent. `mkdir -p` accepts an existing symlink-to-dir + # silently and reports success, after which the move follows it out of the + # tree entirely. + if [ -L "$dir/.idd" ] || { [ -e "$dir/.idd" ] && [ ! -d "$dir/.idd" ]; }; then + echo " ✗ .claude/.idd is not a directory (symlink or other node) — refusing: $repo" >&2 + failed=$((failed + 1)); continue + fi + + # The destination itself. + if [ -L "$current" ]; then + echo " ✗ .idd/local.json is a symlink — refusing: $repo" >&2 + failed=$((failed + 1)); continue + fi + if [ -e "$current" ] && [ ! -f "$current" ]; then + # A directory here is the quiet one: `mv` would move the file INSIDE it + # and report success, leaving .idd/local.json/issue-driven-dev.local.json. + echo " ✗ .idd/local.json exists but is not a regular file — refusing: $repo" >&2 + failed=$((failed + 1)); continue + fi + if [ -f "$current" ]; then # Both exist. The reader already prefers the current path, so moving would # change nothing and could destroy a hand-edited legacy file. Report, do @@ -83,27 +112,55 @@ for root in "${ROOTS[@]}"; do if ! mkdir -p "$dir/.idd" 2>/dev/null; then echo " ✗ cannot create $dir/.idd" >&2; failed=$((failed + 1)); continue fi - if ! mv "$legacy" "$current" 2>/dev/null; then - echo " ✗ move failed: $legacy" >&2; failed=$((failed + 1)); continue + # Re-check after creating it: the window between the test above and here is + # small, but the whole point of these guards is that something else may be + # writing into the same tree. + if [ -L "$dir/.idd" ] || [ ! -d "$dir/.idd" ]; then + echo " ✗ .claude/.idd changed shape while migrating — refusing: $repo" >&2 + failed=$((failed + 1)); continue + fi + + # `mv` has no atomic no-clobber: the check above and the rename below are + # two separate syscalls, and plain rename(2) replaces whatever is at the + # destination. `ln` DOES fail atomically when the destination exists, so a + # hardlink-then-unlink pair gives the guarantee `mv` cannot. It also fails + # safe in the middle: an interrupted migration leaves two links to the same + # inode, i.e. the config still readable at both paths, never at neither. + # (`mv -n` was the other candidate; its exit status does not distinguish + # "skipped because the destination existed" from "moved" on BSD.) + if ! ln "$legacy" "$current" 2>/dev/null; then + echo " ✗ could not link $legacy -> $current (destination appeared, or a" >&2 + echo " filesystem boundary sits between them) — left in place" >&2 + failed=$((failed + 1)); continue + fi + if ! rm -f "$legacy" 2>/dev/null; then + echo " ⚠ copied to $current but could not remove the legacy path: $legacy" >&2 + failed=$((failed + 1)); continue fi # Leave a breadcrumb: a repo whose config silently relocated is confusing to # anyone who bookmarked the old path or greps for it. - # Never truncate an existing file: the breadcrumb is a courtesy, not a - # reason to destroy something a user put there. - if [ -e "$legacy.moved" ]; then - echo " note: $legacy.moved already exists — breadcrumb not written" >&2 - else - printf '%s\n' \ + # Never truncate an existing file, and never write THROUGH a symlink: the + # breadcrumb is a courtesy, not a reason to destroy something a user put + # there. -L is tested separately because a dangling link is invisible to -e, + # and redirection into one CREATES the target. + if [ -L "$legacy.moved" ] || [ -e "$legacy.moved" ]; then + echo " note: $legacy.moved already exists (or is a symlink) — breadcrumb not written" >&2 + elif ! printf '%s\n' \ "This file moved to .claude/.idd/local.json (#303, $(date +%Y-%m-%d))." \ "The old path is no longer written by any IDD skill." \ - > "$legacy.moved" + > "$legacy.moved" 2>/dev/null; then + echo " note: breadcrumb write failed: $legacy.moved" >&2 fi echo " ✓ migrated: $repo" migrated=$((migrated + 1)) + # `-path` and not `-name`: the name alone matches a doc sample, a fixture, a + # test payload — anywhere in any repo. Migrating one of those invents a + # `.idd` directory in a tree that has nothing to do with IDD config. Only a + # file sitting directly in a `.claude/` directory is config. done < <(find "$root" \ \( -name node_modules -o -name .git -o -name .build -o -name .venv \ -o -name archive -o -name archived -o -path '*/.claude/worktrees' \) -prune -o \ - -type f -name "$LEGACY_NAME" -print0 2>/dev/null) + -type f -path "*/.claude/$LEGACY_NAME" -print0 2>/dev/null) done echo "" diff --git a/plugins/issue-driven-dev/scripts/tests/migrate-idd-config/test.sh b/plugins/issue-driven-dev/scripts/tests/migrate-idd-config/test.sh index 3f5a4aa..f3877c4 100755 --- a/plugins/issue-driven-dev/scripts/tests/migrate-idd-config/test.sh +++ b/plugins/issue-driven-dev/scripts/tests/migrate-idd-config/test.sh @@ -39,5 +39,109 @@ require "its legacy sibling is also left in place" \ require "node_modules config was not touched" \ test -f "$S/n/node_modules/x/.claude/issue-driven-dev.local.json" +# ───────────────────────────────────────────────────────────────────────────── +# Hostile tree. Everything below is a shape the script must REFUSE rather than +# handle: the destination or the breadcrumb path is not the ordinary regular +# file the happy path assumes. These live in their own root and get their own +# --apply run, because a refusal is a failure (exit 1) and mixing them into the +# tree above would make the "apply exits 0" assertion vacuous. +# +# Every case here is a real filesystem shape an attacker (or a careless symlink +# farm) can plant inside a repo you scan. `--apply` defaults to walking ALL of +# ~/Developer, so "a repo I cloned" is inside the threat model. +# ───────────────────────────────────────────────────────────────────────────── +H=$(mktemp -d); OUTSIDE=$(mktemp -d) +trap 'rm -rf "$S" "$H" "$OUTSIDE"' EXIT +L="issue-driven-dev.local.json" + +# H1 — .idd is a symlink to a directory outside the repo. `mkdir -p` accepts an +# existing symlink-to-dir silently, so the move follows it and the config lands +# outside the tree the user was told it would stay in. +mkdir -p "$H/h1/.claude" "$OUTSIDE/exfil" +echo '{"github_repo":"o/h1"}' > "$H/h1/.claude/$L" +ln -s "$OUTSIDE/exfil" "$H/h1/.claude/.idd" + +# H2 — the breadcrumb path is a symlink to a file the user cares about. `>` and +# `>>` both follow symlinks, so writing the breadcrumb truncates the target. +mkdir -p "$H/h2/.claude" +echo '{"github_repo":"o/h2"}' > "$H/h2/.claude/$L" +printf 'precious\n' > "$OUTSIDE/victim.txt" +ln -s "$OUTSIDE/victim.txt" "$H/h2/.claude/$L.moved" + +# H2b — the same, but DANGLING. `[ -e ]` is false for a broken link, so the +# existing "already exists" guard does not fire and `>` CREATES the target. +mkdir -p "$H/h2b/.claude" +echo '{"github_repo":"o/h2b"}' > "$H/h2b/.claude/$L" +ln -s "$OUTSIDE/planted.txt" "$H/h2b/.claude/$L.moved" + +# H3 — the destination exists as a DIRECTORY. `[ -f ]` is false for it, so the +# both-present branch is skipped and `mv` moves the file INSIDE it, producing +# .idd/local.json/issue-driven-dev.local.json while reporting "✓ migrated". +mkdir -p "$H/h3/.claude/.idd/local.json" +echo '{"github_repo":"o/h3"}' > "$H/h3/.claude/$L" + +# H4 — the destination is a DANGLING symlink. `[ -f ]` is false (it resolves to +# nothing), so nothing warns; rename() then replaces the link itself. +mkdir -p "$H/h4/.claude/.idd" +echo '{"github_repo":"o/h4"}' > "$H/h4/.claude/$L" +ln -s "$OUTSIDE/does-not-exist" "$H/h4/.claude/.idd/local.json" + +# H4b — the destination is a symlink to an existing regular file elsewhere. +# `-f` FOLLOWS it, so without an -L test this reports "both present, current +# wins" — an audit line that says the config is in the repo when it is not. +mkdir -p "$H/h4b/.claude/.idd" +echo '{"github_repo":"o/h4b"}' > "$H/h4b/.claude/$L" +echo '{"github_repo":"o/elsewhere"}' > "$OUTSIDE/other-config.json" +ln -s "$OUTSIDE/other-config.json" "$H/h4b/.claude/.idd/local.json" + +# H5 — a file with the legacy NAME that is not in a `.claude/` directory at all. +# It is not IDD config; migrating it invents a `.idd` directory in someone +# else's tree. The find predicate is name-only, so it matches. +mkdir -p "$H/h5/docs/examples" +echo '{"github_repo":"o/h5-doc-sample"}' > "$H/h5/docs/examples/$L" + +# H6 — the legacy path is itself a symlink. Regression lock, not a new guard: +# `find -type f` uses lstat, so a symlink is already excluded. Pinned because +# the guard is invisible (it lives in a flag, not in a line of code) and a +# future switch to `find -L` would silently start following it. +mkdir -p "$H/h6/.claude" +echo 'secret' > "$OUTSIDE/linked-config.json" +ln -s "$OUTSIDE/linked-config.json" "$H/h6/.claude/$L" + +HOUT=$(bash "$SCRIPT" --apply "$H" 2>&1); HRC=$? + +assert_exit "hostile apply exits non-zero (refusals are failures)" 1 "$HRC" + +require "H1 .idd symlink: legacy config stays put" test -f "$H/h1/.claude/$L" +refute "H1 .idd symlink: nothing was written through the link" \ + test -e "$OUTSIDE/exfil/local.json" +assert_grep "H1 .idd symlink: refusal is reported" "not a directory" "$HOUT" + +assert_eq "H2 breadcrumb symlink: victim file is not truncated" \ + "precious" "$(cat "$OUTSIDE/victim.txt")" + +refute "H2b dangling breadcrumb symlink: nothing was created through it" \ + test -e "$OUTSIDE/planted.txt" + +require "H3 destination is a directory: legacy config stays put" test -f "$H/h3/.claude/$L" +refute "H3 destination is a directory: nothing was moved inside it" \ + test -e "$H/h3/.claude/.idd/local.json/$L" + +require "H4 dangling destination symlink: legacy config stays put" test -f "$H/h4/.claude/$L" +require "H4 dangling destination symlink: the link itself is untouched" \ + test -L "$H/h4/.claude/.idd/local.json" + +require "H4b symlinked destination: legacy config stays put" test -f "$H/h4b/.claude/$L" +assert_grep "H4b symlinked destination: refused, NOT called 'both present'" \ + ".idd/local.json is a symlink" "$HOUT" +refute_grep "H4b symlinked destination: no misleading both-present line" \ + "both present, current wins (left alone): $H/h4b" "$HOUT" + +require "H5 legacy name outside .claude/ is not migrated" test -f "$H/h5/docs/examples/$L" +refute "H5 no .idd directory was invented next to it" test -e "$H/h5/docs/examples/.idd" + +require "H6 symlinked legacy path is not followed" test -L "$H/h6/.claude/$L" +assert_eq "H6 its target is unchanged" "secret" "$(cat "$OUTSIDE/linked-config.json")" + print_summary "migrate-idd-config" exit $? From 5466520fac773fd3f3a09e0b8ba4cd90b1a50a41 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:23:14 +0800 Subject: [PATCH 02/11] =?UTF-8?q?fix:=20repo-map=20=E7=9A=84=E5=88=97?= =?UTF-8?q?=E7=B7=A9=E8=A1=9D=E6=94=B9=E7=94=A8=E7=9C=9F=E7=9A=84=E5=AE=9A?= =?UTF-8?q?=E4=BD=8D/=E6=8F=9B=E8=A1=8C=E5=AD=97=E5=85=83=EF=BC=8C?= =?UTF-8?q?=E4=B8=A6=E6=AA=A2=E6=9F=A5=20find=20=E7=9A=84=E9=80=80?= =?UTF-8?q?=E5=87=BA=E7=A2=BC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rows` 用 `\t` / `\n` 兩字元跳脫存放、再用 `printf '%b'` 印出，而 `%b` 也會 展開**資料裡**的跳脫。所以路徑或 `github_repo` 裡的一個反斜線是被執行、 不是被印出。sanitizer 擋掉了控制字元與 bidi mark，從來沒碰過反斜線——當時 下游沒有東西會解讀它。 `\c` 最糟：`%b` 遇到它會停掉**全部**輸出。一個被構造的值就能安靜刪掉它後面 的每一列與 totals 行，而少了一半列的地圖，讀起來跟「這台機器本來就只有這些 repo」一模一樣。`\n` 造出獨立的偽列，`\t` 位移欄位。 **更正一項我在 audit 裡寫錯的話**：當時我說「`--json` 路徑是安全的（jq 會 正確跳脫）」。那句話只對 newline payload 成立。同一個 `%b` 也餵給 jq，所以 反斜線 payload 一樣會截斷 JSON 那一列（實測 `config_format` 變成 `null`）。 我測了我寫的那段（sanitizer），沒測它與 printf 相接的那個縫。 改法：列緩衝直接放真的 TAB / NEWLINE，三處 `%b` 改 `%s`。資料不可能再帶回 真的控制字元，因為 sanitize_field 會刪掉 tab、把 newline 折成空白。 `find` 的退出碼：process substitution 會丟掉它。這件事在別的掃描器只是少幾 列，在這裡是要害——這張地圖的全部工作就是回答「這一層在不在」，而一個讀不 到的目錄回答「這裡沒有 config」，跟真的沒有 config **無法區分**，那正是 #301 的錯誤向上解析。改成先導向暫存檔、檢查退出碼、不完整時明講。順帶把 `-print` 換成 `-print0`（同 #319 對 migrate 的處置，消掉含換行目錄名那一類）。 新增 `refute_grep_re`（+ 自身 3 條測試）。這個 helper 值得存在，是因為固定 字串的否定斷言是本 repo 最容易寫錯的一種：`o/FORGED-ROW` 合法地出現在它被 寫進去的那一列**之內**，`refute_grep` 會因為一個測試根本沒打算檢查的理由而 失敗。要錨定就需要 regex。 測試自身也修過一次：第一版用 printf 寫 payload，檔案裡是 `\c`——那不是合法 的 JSON 跳脫，jq 直接拒收，值根本沒進到列緩衝，於是**探針通過、卻什麼都沒 測到**。改用 quoted heredoc 讓磁碟上真的是 `\\`，並加一條 probe self-check 斷言 payload 確實活著到 jq 之後。 acid：`%s`→`%b`（含還原跳脫式列緩衝）紅 2 條；拿掉 find 退出碼檢查紅 1 條。 全 suite 47/47。 --- .../issue-driven-dev/scripts/idd-repo-map.sh | 43 ++++++++++--- .../scripts/lib/assert-helpers.sh | 7 ++ .../scripts/tests/assert-helpers/test.sh | 19 ++++++ .../scripts/tests/idd-repo-map/test.sh | 64 +++++++++++++++++++ 4 files changed, 123 insertions(+), 10 deletions(-) diff --git a/plugins/issue-driven-dev/scripts/idd-repo-map.sh b/plugins/issue-driven-dev/scripts/idd-repo-map.sh index f629f53..bca4b2e 100755 --- a/plugins/issue-driven-dev/scripts/idd-repo-map.sh +++ b/plugins/issue-driven-dev/scripts/idd-repo-map.sh @@ -69,25 +69,48 @@ emit() { # $1=repo_dir $2=config_path $3=format # rather than letting an empty string read as a match. [ -z "$(printf '%s' "$slug" | tr -d '[:space:]')" ] && slug="(no github_repo)" dir=$(printf '%s' "$dir" | sanitize_field) - rows="${rows}${dir}\t${slug}\t${fmt}\n" + # REAL tab and newline, not the two-character escapes `\t` / `\n`. The buffer + # used to hold escapes and be rendered with `printf '%b'`, which expands + # escapes in the DATA as well: a backslash inside a directory name or a + # `github_repo` value was executed rather than printed. `\c` was the worst of + # them — %b stops ALL output at it, so one crafted value silently deleted + # every row after it AND the totals line, and a map missing half its rows + # reads exactly like a machine with half as many repos. (`--json` was not + # exempt: the same %b feeds jq, so the payload truncated the JSON row too.) + # Real control characters cannot come back from the data, because + # sanitize_field deletes tabs and folds newlines to spaces. + rows="${rows}${dir}"$'\t'"${slug}"$'\t'"${fmt}"$'\n' } +SCAN_INCOMPLETE=0 for root in "${ROOTS[@]}"; do [ -d "$root" ] || continue - while IFS= read -r cfg; do + # `find`'s exit status is the only signal that part of the tree was + # unreadable, and a process substitution throws it away. That matters more + # here than in a normal scanner: this map's entire job is to answer "does this + # layer exist?", and an unreadable directory answering "no config here" is + # indistinguishable from "no config here" — which is precisely the wrong + # upward resolution #301 was filed for. So: capture, then check. + FIND_OUT=$(mktemp) || continue + find "$root" \ + \( -name node_modules -o -name .git -o -name .build -o -name .venv \ + -o -name archive -o -name archived -o -path '*/.claude/worktrees' \) -prune -o \ + \( -path '*/.claude/.idd/local.json' -o -path '*/.claude/issue-driven-dev.local.json' \) \ + -print0 >"$FIND_OUT" 2>/dev/null + [ $? -eq 0 ] || SCAN_INCOMPLETE=1 + while IFS= read -r -d '' cfg; do case "$cfg" in */.claude/.idd/local.json) emit "$(dirname "$(dirname "$(dirname "$cfg")")")" "$cfg" current ;; */.claude/issue-driven-dev.local.json) emit "$(dirname "$(dirname "$cfg")")" "$cfg" legacy ;; esac - done < <(find "$root" \ - \( -name node_modules -o -name .git -o -name .build -o -name .venv \ - -o -name archive -o -name archived -o -path '*/.claude/worktrees' \) -prune -o \ - \( -path '*/.claude/.idd/local.json' -o -path '*/.claude/issue-driven-dev.local.json' \) \ - -print 2>/dev/null) + done < "$FIND_OUT" + rm -f "$FIND_OUT" done +[ "$SCAN_INCOMPLETE" -eq 0 ] || \ + echo "note: parts of the scanned tree could not be read (permissions?) — this map may be INCOMPLETE, and a missing row means 'no such layer' to every consumer of it." >&2 if [ "$JSON" = "1" ]; then - printf '%b' "$rows" | jq -R -s --arg g "$GLOBAL" ' + printf '%s' "$rows" | jq -R -s --arg g "$GLOBAL" ' {global: ($g | if (. | test("^/")) then . else null end), global_present: false, repos: (split("\n") | map(select(length > 0) | split("\t") @@ -102,8 +125,8 @@ if [ -z "$rows" ]; then echo "no IDD-configured repo found under: ${ROOTS[*]}" exit 0 fi -printf '%b' "$rows" | sort | awk -F'\t' '{printf " %-58s %-38s %s\n", $1, $2, $3}' +printf '%s' "$rows" | sort | awk -F'\t' '{printf " %-58s %-38s %s\n", $1, $2, $3}' echo "" -printf '%b' "$rows" | awk -F'\t' '{c[$3]++} END {printf "total: %d (current: %d, legacy: %d)\n", NR, c["current"], c["legacy"]}' +printf '%s' "$rows" | awk -F'\t' '{c[$3]++} END {printf "total: %d (current: %d, legacy: %d)\n", NR, c["current"], c["legacy"]}' echo "(legacy rows are migratable — see scripts/migrate-idd-config.sh, #303)" exit 0 diff --git a/plugins/issue-driven-dev/scripts/lib/assert-helpers.sh b/plugins/issue-driven-dev/scripts/lib/assert-helpers.sh index f4b170d..7fb6bf8 100755 --- a/plugins/issue-driven-dev/scripts/lib/assert-helpers.sh +++ b/plugins/issue-driven-dev/scripts/lib/assert-helpers.sh @@ -63,6 +63,13 @@ refute_grep() { # name needle haystack assert_grep_re() { # name ere_pattern haystack if printf '%s\n' "$3" | grep -qE -- "$2"; then pass "$1"; else fail "$1" "pattern not matched: [$2]"; fi } +# The negative form. Worth having as a helper rather than an inline `! grep`, +# because a fixed-string refutation is the easiest assertion in this repo to get +# wrong: a payload that legitimately appears INSIDE a line makes `refute_grep` +# fail for a reason the test did not intend to check. Anchoring needs a regex. +refute_grep_re() { # name ere_pattern haystack + if printf '%s\n' "$3" | grep -qE -- "$2"; then fail "$1" "pattern unexpectedly matched: [$2]"; else pass "$1"; fi +} # ── output-file grep family (#188 — the safe form for captured output) ── # assert_output_grep : file MUST contain needle (fixed-string, `--`-safe). diff --git a/plugins/issue-driven-dev/scripts/tests/assert-helpers/test.sh b/plugins/issue-driven-dev/scripts/tests/assert-helpers/test.sh index 9a09a29..82b0dd4 100755 --- a/plugins/issue-driven-dev/scripts/tests/assert-helpers/test.sh +++ b/plugins/issue-driven-dev/scripts/tests/assert-helpers/test.sh @@ -35,4 +35,23 @@ fi # 5. header documents the eval-content ban assert_output_grep "header carries eval-content warning" 'never interpolate captured output' "$HERE/../../lib/assert-helpers.sh" +# 6. refute_grep_re anchors where refute_grep cannot. The pair below is the +# whole reason the helper exists: `marker` occurs inside a line, so a +# fixed-string refutation fails for a reason the test never meant to assert. +HAY=' o/real-repo o/FORGED-ROW current' +refute_grep_re "refute_grep_re: an in-line occurrence does not match an anchored pattern" \ + '^ o/FORGED-ROW' "$HAY" +assert_grep_re "assert_grep_re: the same string IS found unanchored" \ + 'o/FORGED-ROW' "$HAY" + +# 7. and it still fails when the pattern really does match +RG_BEFORE=$FAIL +refute_grep_re "anchored-match probe" '^ o/real-repo' "$HAY" 2>/dev/null || true +if [ "$FAIL" -gt "$RG_BEFORE" ]; then + FAIL=$((FAIL - 1)); unset 'FAILURES[${#FAILURES[@]}-1]' 2>/dev/null + pass "refute_grep_re: a genuine anchored match fails loudly" +else + fail "refute_grep_re: a genuine anchored match fails loudly" "helper passed when it should have failed" +fi + print_summary "assert-helpers" diff --git a/plugins/issue-driven-dev/scripts/tests/idd-repo-map/test.sh b/plugins/issue-driven-dev/scripts/tests/idd-repo-map/test.sh index cd8431d..be2a0e3 100755 --- a/plugins/issue-driven-dev/scripts/tests/idd-repo-map/test.sh +++ b/plugins/issue-driven-dev/scripts/tests/idd-repo-map/test.sh @@ -39,5 +39,69 @@ OUT2=$(bash "$SCRIPT" "$S/does-not-exist" 2>&1) assert_grep "a missing root yields an explicit no-repo line, not silence" \ "no IDD-configured repo found" "$OUT2" +# ───────────────────────────────────────────────────────────────────────────── +# Backslashes. The row buffer used `\t` / `\n` as two-character escapes and was +# rendered with `printf '%b'`, which expands escapes in the DATA too — so a +# backslash in a directory name or a github_repo value was executed rather than +# printed. The sanitiser strips control characters and bidi marks; it never +# touched backslashes, because at the time nothing downstream interpreted them. +# ───────────────────────────────────────────────────────────────────────────── +B=$(mktemp -d); trap 'rm -rf "$S" "$B"' EXIT + +# The payloads must reach the row buffer as LITERAL backslashes, so the JSON on +# disk carries `\\`. Written via a quoted heredoc: an earlier version of this +# test used printf and produced `\c` in the file, which is not a legal JSON +# escape — jq rejected it, the value never reached the row buffer, and the probe +# passed while testing nothing. +mk() { mkdir -p "$B/$1/.claude/.idd"; cat > "$B/$1/.claude/.idd/local.json"; } + +# `\c` is the dangerous one: %b stops ALL output at it. Everything after this +# row — every other repo, the totals line — silently disappears, and a map that +# lost half its rows reads exactly like a machine with half as many repos. +mk z-tail <<'EOF' +{"github_repo":"o/tail-marker"} +EOF +mk a-trunc <<'EOF' +{"github_repo":"o/x\\ceaten"} +EOF +# `\n` forges a standalone row; `\t` shifts the columns of the row it is in. +mk b-forge <<'EOF' +{"github_repo":"o/y\\no/FORGED-ROW"} +EOF +mk c-tab <<'EOF' +{"github_repo":"o/z\\tINJECTED"} +EOF + +require "the backslash payload survived as JSON (probe self-check)" \ + bash -c 'jq -e -r ".github_repo" "$0" | grep -q "x.ceaten"' "$B/a-trunc/.claude/.idd/local.json" + +BOUT=$(bash "$SCRIPT" "$B" 2>/dev/null) +assert_grep "a backslash-c value does not truncate the rest of the map" \ + "tail-marker" "$BOUT" +assert_grep "totals line still printed after a backslash-c value" "total:" "$BOUT" +# Precise, not incidental: `o/FORGED-ROW` legitimately appears INSIDE the row it +# was written into. What must never happen is it becoming a row of its own, i.e. +# occupying the first column. Grepping for the bare string tests neither. +refute_grep_re "a backslash-n value does not forge a standalone row" \ + '^ o/FORGED-ROW' "$BOUT" +assert_eq "row count is unchanged by backslash payloads" \ + "4" "$(printf '%s' "$BOUT" | grep -c '^ ')" + +BJSON=$(bash "$SCRIPT" --json "$B" 2>/dev/null) +require "--json stays parseable with backslash payloads" \ + bash -c 'printf "%s" "$0" | jq -e . >/dev/null' "$BJSON" +require "--json reports exactly the four real repos" \ + bash -c '[ "$(printf "%s" "$0" | jq ".repos | length")" = "4" ]' "$BJSON" + +# An unreadable directory must not read as "no config here" — that is the exact +# failure the map exists to prevent (an absent layer resolves upward, wrongly). +U=$(mktemp -d); mkdir -p "$U/locked/deep" +chmod 000 "$U/locked" +UOUT=$(bash "$SCRIPT" "$U" 2>&1); URC=$? +chmod 755 "$U/locked"; rm -rf "$U" +assert_exit "an unreadable subtree still exits 0 (advisory contract)" 0 "$URC" +assert_grep "an unreadable subtree is reported, not silently mapped as empty" \ + "could not be read" "$UOUT" + print_summary "idd-repo-map" exit $? From e9ad88b51c6482785f338a58491609618e903990 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:31:57 +0800 Subject: [PATCH 03/11] =?UTF-8?q?feat:=20--retroactive=20=E7=9A=84=20preco?= =?UTF-8?q?ndition=20=E6=94=B9=E6=88=90=E7=9C=9F=E7=9A=84=E5=9F=B7?= =?UTF-8?q?=E8=A1=8C=20classifier=EF=BC=88--issue=20N=20gate=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `idd-close/SKILL.md` 原本自己寫著：「本 gate 是散文，不是機械強制…機械判定 只存在於 check-closed-without-summary.sh，而本 skill **並未呼叫它**…未做」。 揭露是誠實的，但揭露不是緩解——**七輪 verify 的全部成果，要等 agent 剛好讀到 那張表才生效**，而它守的是一個不可逆動作（在可能已有 summary 的 issue 上再 貼一份）。 helper 新增 `--issue N` 單一 issue 模式，輸出一個 JSON 物件，並且**用退出碼 下判決**： 0 class == missing 且 comment 集合完整 → 唯一放行 1 其他分類（compliant / casing / present） → 拒絕 2 無法判定（未 CLOSED / 截斷 / 抓取或解析失敗）→ 拒絕 審計模式的「永遠 exit 0」契約原樣不動（有斷言釘住）。那條契約是對**人**報告 用的；gate 是不可逆動作的前置條件，一個永遠 exit 0 的 gate 不是 gate——呼叫端 只能去解讀散文，而那正是這七輪維持 advisory 的原因。 gate 模式另外**不走** `--json comments` 那條路：它用 REST `--paginate` 抓 comment。那條巢狀 connection 硬上限 100 則且回**最舊**的 100 則，而 closing summary 依定義是**最新**一則。審計端是事後修補截斷（#319），gate 端根本不走。 fail-closed 的範圍寫死在 skill 裡，包含「helper 不在」——找不到 gate 等於沒有 gate，不是「那就跳過吧」。 prose-drift 測試補兩類斷言：(1) skill 必須真的**執行** helper（比對呼叫式與 退出碼分支），不是只「指向 normative source」——指向正是它散文化七輪期間一直 在做的事；(2) helper 必須真的有那個 flag，否則 skill 呼叫一個不存在的旗標會 以最壞的方式 fail open（未知旗標只警告不中止，等於把審計的 always-exit-0 搬到破壞性路徑上）。 順帶修掉 drift 測試的 canary：它往 repo 寫固定檔名、沒有 trap。中途被打斷就 留下一個檔案，之後每一次執行都紅、且會被 `git add -A` 帶進 commit；固定檔名 還讓兩個並行執行互刪對方的 canary，各自把對方的刪除讀成「掃描看不到植入的 字串」。改成唯一檔名 + trap。 測試自身修過兩次： - `jq -r ".comments_complete // \"null\""` 把真正的 `false` 報成 "null" （jq 的 `//` 視 false 為空），那條截斷斷言因此不可能失敗。改 `tostring`。 - 一個斷言字串裡的反引號被 shell 當成命令替換執行掉了。 acid：G1 截斷仍放行→紅 2；G2 非 missing 仍放行→紅 4；G3 未 CLOSED 不再拒絕 →紅 1；G5 skill 不再呼叫 gate→紅 1。G4（--issue 整數驗證）第一輪**沒有測試 重量**——拿掉它 `--issue abc` 仍然 exit 2，只是改由下游 jq 崩潰造成，退出碼 分不出兩者。補一條斷言檢查錯誤訊息確實來自驗證本身後才轉紅（那條驗證真正 要守的是 live 路徑：數字會被插進 API URL，中間沒有 jq 擋著）。 全 suite 47/47，classifier suite 114 assertions。 --- .../scripts/check-closed-without-summary.sh | 104 +++++++++++++++++- .../check-closed-without-summary/test.sh | 64 +++++++++++ .../tests/closing-summary-prose-drift/test.sh | 40 ++++++- .../skills/idd-close/SKILL.md | 27 ++++- 4 files changed, 229 insertions(+), 6 deletions(-) diff --git a/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh b/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh index dae61b9..ad8b2ed 100755 --- a/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh +++ b/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh @@ -20,11 +20,27 @@ # heading and a real one count the same. That under-reports on purpose — see the # rationale above CLASSIFY. # -# Advisory only — ALWAYS exits 0. +# Advisory only in AUDIT mode — it ALWAYS exits 0 there. +# +# `--issue N` is the exception, and deliberately so. Audit mode reports to a +# human; `--issue N` is a GATE for `/idd-close --retroactive`, whose action is +# irreversible (it posts a second summary onto an issue that may already have +# one). A gate that always exits 0 is not a gate — the caller has to interpret +# prose, which is how seven rounds of work on this classifier stayed advisory +# while the destructive path went on deciding for itself. # # Usage: # check-closed-without-summary.sh [--repo owner/repo] [--limit N] [--since YYYY-MM-DD] # check-closed-without-summary.sh --json-file # test / offline mode +# check-closed-without-summary.sh --issue N [--repo …] # single-issue GATE +# +# `--issue N` prints one JSON object and exits: +# 0 class == missing, comment set known complete -> --retroactive may run +# 1 any other class -> refuse, it has one +# 2 could not determine (not closed / truncated / -> refuse +# fetch or parse failure / no such issue) +# Everything that is not a confident `missing` refuses. Fail-closed is the only +# safe default when the action cannot be undone. # # Consumed by idd-list `--audit-closes`. The `## Closing Summary` heading is the # same marker idd-list Step 3 keys on for phase inference. @@ -36,6 +52,8 @@ REPO="" LIMIT=50 SINCE="" DRY_RUN=0 +GATE_ISSUE="" +GATE_ERR="" while [ $# -gt 0 ]; do case "$1" in @@ -43,6 +61,7 @@ while [ $# -gt 0 ]; do # trailing value-taking flag looped forever — the advisory contract promises # exit 0, and never exiting breaks it harder than any wrong verdict. --json-file) JSON_FILE="${2:-}"; shift; [ $# -gt 0 ] && shift ;; + --issue) GATE_ISSUE="${2:-}"; shift; [ $# -gt 0 ] && shift ;; --repo) REPO="${2:-}"; shift; [ $# -gt 0 ] && shift ;; --limit) LIMIT="${2:-50}"; shift; [ $# -gt 0 ] && shift ;; --since) SINCE="${2:-}"; shift; [ $# -gt 0 ] && shift ;; @@ -55,10 +74,44 @@ while [ $# -gt 0 ]; do esac done +# ── Gate mode plumbing (--issue N) ── +# One JSON object on stdout, and an exit code the caller cannot misread. Every +# path that is not a confident `missing` on a complete comment set exits 2 (or +# 1), because the caller is about to do something irreversible. +gate_out() { # $1=class-or-empty $2=state-or-empty $3=complete(true/false) $4=error-or-empty $5=exit code + jq -n --arg n "$GATE_ISSUE" --arg c "${1:-}" --arg s "${2:-}" \ + --argjson complete "${3:-false}" --arg e "${4:-}" \ + '{number: ($n | tonumber? // null), + state: (if $s == "" then null else $s end), + class: (if $c == "" then null else $c end), + comments_complete: $complete, + error: (if $e == "" then null else $e end)}' + exit "$5" +} +if [ -n "$GATE_ISSUE" ]; then + # Validated before it is interpolated into an API path, and before anything + # downstream compares it numerically. + case "$GATE_ISSUE" in + ''|*[!0-9]*) gate_out "" "" false "--issue expects an integer issue number" 2 ;; + esac +fi + # ── Acquire issue JSON ── if [ -n "$JSON_FILE" ]; then - [ -f "$JSON_FILE" ] || { echo "✗ --json-file not found: $JSON_FILE" >&2; exit 0; } + if [ ! -f "$JSON_FILE" ]; then + [ -n "$GATE_ISSUE" ] && gate_out "" "" false "--json-file not found: $JSON_FILE" 2 + echo "✗ --json-file not found: $JSON_FILE" >&2; exit 0 + fi ISSUES_JSON=$(cat "$JSON_FILE") + if [ -n "$GATE_ISSUE" ]; then + # Narrow to the one issue. An offline payload carries whatever comments the + # fixture author put there, so completeness is whatever the fixture says. + ISSUES_JSON=$(printf '%s' "$ISSUES_JSON" \ + | jq --argjson n "$GATE_ISSUE" '[.[] | select(.number == $n)]' 2>/dev/null) \ + || gate_out "" "" false "could not read the offline payload" 2 + [ "$(printf '%s' "$ISSUES_JSON" | jq 'length' 2>/dev/null)" = "1" ] \ + || gate_out "" "" false "issue #$GATE_ISSUE is not in the payload" 2 + fi else # Resolve repo: --repo flag → walk-up .claude/.idd config → gh default repo. if [ -z "$REPO" ]; then @@ -88,6 +141,26 @@ else [ -n "$REPO" ] && echo "note: repo resolved from the global layer ($REPO) — no repo-local config found." >&2 fi fi + if [ -n "$GATE_ISSUE" ]; then + # Gate mode fetches ONE issue, and fetches its comments through REST with + # --paginate rather than the nested `--json comments` connection. That is + # not a preference: the nested form is hard-capped at the OLDEST 100, and a + # closing summary is by construction the NEWEST comment. The audit path + # repairs that after the fact; the gate simply never takes the broken road. + GATE_REPO="$REPO" + [ -z "$GATE_REPO" ] && GATE_REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null) + [ -n "$GATE_REPO" ] || gate_out "" "" false "could not resolve the target repo" 2 + META=$(gh issue view "$GATE_ISSUE" --repo "$GATE_REPO" --json number,title,state 2>/dev/null) \ + || gate_out "" "" false "could not fetch issue #$GATE_ISSUE from $GATE_REPO" 2 + if ! CMTS=$(gh api "repos/$GATE_REPO/issues/$GATE_ISSUE/comments" --paginate \ + --jq '[.[] | {body}]' 2>/dev/null | jq -s 'add // []' 2>/dev/null); then + gate_out "" "" false "could not fetch the comments of #$GATE_ISSUE" 2 + fi + printf '%s' "$CMTS" | jq -e 'type == "array"' >/dev/null 2>&1 \ + || gate_out "" "" false "the comment fetch returned something that is not an array" 2 + ISSUES_JSON=$(printf '%s' "$META" | jq --argjson c "$CMTS" '[. + {comments: $c}]' 2>/dev/null) \ + || gate_out "" "" false "could not assemble the issue payload" 2 + else GH_ARGS=(issue list --state closed --json number,title,state,comments --limit "$LIMIT") [ -n "$REPO" ] && GH_ARGS+=(--repo "$REPO") [ -n "$SINCE" ] && GH_ARGS+=(--search "closed:>=$SINCE") @@ -168,17 +241,22 @@ else fi done fi + fi fi # Fail-safe: if the acquired payload is NOT valid JSON (e.g. gh returned 0 with a # truncated stream / proxy HTML, or a hand-edited fixture is malformed), do NOT # fall through to the filter and print a false "✓ all-clear" — that's the worst # direction for a safety-net audit (false reassurance). Warn + exit, no verdict. +# In gate mode the same conditions must exit 2, not 0: `exit 0` there would +# read as "confirmed missing, go ahead and post". if [ -z "$(printf '%s' "$ISSUES_JSON" | tr -d '[:space:]')" ]; then + [ -n "$GATE_ISSUE" ] && gate_out "" "" false "issue payload is empty" 2 echo "note: issue payload is empty — audit skipped, no conclusion drawn." >&2 exit 0 fi if ! printf '%s' "$ISSUES_JSON" | jq -e 'type == "array"' >/dev/null 2>&1; then + [ -n "$GATE_ISSUE" ] && gate_out "" "" false "issue payload is not a JSON array" 2 echo "note: issue payload is not a JSON array — audit skipped, no conclusion drawn." >&2 exit 0 fi @@ -408,10 +486,32 @@ if ! CLASSIFIED=$(printf '%s' "$ISSUES_JSON" | jq -r "$CLASSIFY" 2>"${JQ_ERR:-/d [ -n "$JQ_ERR" ] && LC_ALL=C tr -d "\000-\010\013\014\016-\037\177" < "$JQ_ERR" \ | head -5 | sed "s/^/ jq: /" >&2 [ -n "$JQ_ERR" ] && rm -f "$JQ_ERR" + [ -n "$GATE_ISSUE" ] && gate_out "" "" false "classification filter failed" 2 exit 0 fi [ -n "$JQ_ERR" ] && rm -f "$JQ_ERR" +# ── Gate verdict ── +# CLASSIFY only emits a row for issues whose state is CLOSED, so an empty result +# here means "not closed" — which is not a retroactive case at all, and is +# exactly the state in which posting a second summary would be worst. +if [ -n "$GATE_ISSUE" ]; then + GATE_STATE=$(printf '%s' "$ISSUES_JSON" | jq -r '.[0].state // ""' 2>/dev/null) + GATE_TRUNC=$(printf '%s' "$ISSUES_JSON" | jq -r 'if .[0].idd_comments_truncated == true then "true" else "false" end' 2>/dev/null) + GATE_CLASS=$(printf '%s\n' "$CLASSIFIED" | awk -F'\t' 'NF { print $1; exit }') + [ -n "$GATE_CLASS" ] || gate_out "" "$GATE_STATE" false \ + "issue #$GATE_ISSUE is not CLOSED — not a retroactive case" 2 + if [ "$GATE_TRUNC" = "true" ]; then + gate_out "$GATE_CLASS" "$GATE_STATE" false \ + "the comment set is known to be incomplete — absence proves nothing" 2 + fi + case "$GATE_CLASS" in + missing) gate_out missing "$GATE_STATE" true "" 0 ;; + *) gate_out "$GATE_CLASS" "$GATE_STATE" true \ + "class is $GATE_CLASS, not missing — this issue already carries a closing-summary marker" 1 ;; + esac +fi + pick() { printf '%s\n' "$CLASSIFIED" | awk -F'\t' -v c="$1" '$1 == c { print $2 }'; } MISSING=$(pick missing) diff --git a/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/test.sh b/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/test.sh index ab4e044..10c6156 100644 --- a/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/test.sh +++ b/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/test.sh @@ -387,5 +387,69 @@ require "prose mentioning the marker still cannot rescue an issue" \ bash -c 'printf "%s" "[{\"number\":9001,\"title\":\"p\",\"state\":\"CLOSED\",\"comments\":[{\"body\":\"I forgot the closing summary, sorry\"}]}]" > "$0/p.json"; bash "$1" --json-file "$0/p.json" | grep -q "9001"' "${TMPDIR:-/tmp}" "$HELPER" +# ── `--issue N`: the single-issue GATE (#307 follow-up) ──────────────────────── +# Audit mode reports to a human and always exits 0. This mode is a precondition +# for an IRREVERSIBLE action, so the whole point is the exit code: the caller +# must not have to read prose to find out whether posting is allowed. +# +# 0 = confident `missing` on a complete comment set 1 = any other class +# 2 = could not determine anything +# +# Everything that is not a confident 0 must refuse. These assertions are the +# only reason the seven rounds of classifier work bind the destructive path at +# all — before this mode existed, idd-close reimplemented the judgement in prose. +gate() { bash "$HELPER" --json-file "$FIXTURE" --issue "$1" 2>/dev/null; } +gate_rc() { gate "$1" >/dev/null 2>&1; echo $?; } +# `tostring`, NOT `// "null"`: in jq the alternative operator treats `false` as +# empty, so `.comments_complete // "null"` reports a genuine `false` as "null" +# — which would have made the truncation assertion below unable to fail. +gate_field() { gate "$1" | jq -r ".$2 | tostring"; } + +assert_eq "gate: a genuinely-missing issue exits 0" "0" "$(gate_rc 101)" +assert_eq "gate: ...and says so in machine-readable form" "missing" "$(gate_field 101 class)" +assert_eq "gate: ...and asserts the comment set was complete" "true" "$(gate_field 101 comments_complete)" +assert_eq "gate: a zero-comment closed issue also exits 0" "0" "$(gate_rc 103)" + +assert_eq "gate: a compliant issue REFUSES (exit 1)" "1" "$(gate_rc 100)" +assert_eq "gate: a casing issue REFUSES — the summary is there, only misspelt" \ + "1" "$(gate_rc 104)" +assert_eq "gate: a PRESENT issue REFUSES — nothing was established about it" \ + "1" "$(gate_rc 110)" +# #155 is a pure QUOTATION. It is the shape the audit deliberately under-reports +# on, and the gate must inherit that: refusing here costs a missed remediation, +# allowing here costs a duplicate post onto someone else's issue. +assert_eq "gate: a quotation-only issue REFUSES rather than authorising a post" \ + "1" "$(gate_rc 155)" + +assert_eq "gate: an OPEN issue exits 2 — not a retroactive case at all" \ + "2" "$(gate_rc 102)" +assert_eq "gate: a truncated comment set exits 2, never 0" "2" "$(gate_rc 158)" +assert_eq "gate: ...and reports the comment set as incomplete" \ + "false" "$(gate_field 158 comments_complete)" +assert_eq "gate: an issue absent from the payload exits 2" "2" "$(gate_rc 9999)" +assert_eq "gate: a non-numeric --issue exits 2" "2" "$(gate_rc abc)" +# ...and exits 2 because the VALIDATOR fired, not because jq happened to choke +# downstream. Deleting the validation leaves the exit code at 2 (the offline +# path dies on --argjson instead), so the code alone cannot tell the two apart — +# and the reason the validation exists is the LIVE path, where the number is +# interpolated into an API URL and no jq stands between it and the request. +assert_grep "gate: ...because the integer check fired, not because jq crashed" \ + "expects an integer" "$(gate_field abc error)" +assert_eq "gate: a malformed payload exits 2, not 0" "2" \ + "$(bash "$HELPER" --json-file "$HERE/fixtures/malformed.json" --issue 101 >/dev/null 2>&1; echo $?)" + +# The output must be a single JSON object on stdout for EVERY path, including +# the failures — a caller that has to distinguish "JSON" from "a sentence" will +# eventually get it wrong. +for n in 101 100 102 158 9999 abc; do + require "gate: --issue $n emits one parseable JSON object" \ + bash -c 'bash "$0" --json-file "$1" --issue "$2" 2>/dev/null | jq -e "type == \"object\"" >/dev/null' \ + "$HELPER" "$FIXTURE" "$n" +done + +# Audit mode must be UNAFFECTED: it still always exits 0, gate or no gate. +assert_eq "audit mode still exits 0 (advisory contract intact)" "0" \ + "$(bash "$HELPER" --json-file "$FIXTURE" >/dev/null 2>&1; echo $?)" + print_summary "check-closed-without-summary" exit $? diff --git a/plugins/issue-driven-dev/scripts/tests/closing-summary-prose-drift/test.sh b/plugins/issue-driven-dev/scripts/tests/closing-summary-prose-drift/test.sh index 8b4c825..f7f46af 100644 --- a/plugins/issue-driven-dev/scripts/tests/closing-summary-prose-drift/test.sh +++ b/plugins/issue-driven-dev/scripts/tests/closing-summary-prose-drift/test.sh @@ -63,7 +63,16 @@ require "no prose file quotes a regex literal for the closing-summary marker" \ # it, then remove it. Without this, a broken scan (bad flag, wrong path, a `--` # swallowing --include — all three happened while writing this file) reads as a # clean repo. -CANARY="$PLUGIN/.drift-canary.md" +# The canary is a file written INTO THE REPO. Two consequences the first cut +# ignored: an interrupted run (Ctrl-C, a failing assertion under `set -e`, a +# killed CI job) leaves it behind, where it is both a permanent red for every +# later run and something a careless `git add -A` will commit; and a fixed name +# means two concurrent runs delete each other's canary and each reads the other's +# removal as "the scan cannot see a planted literal". Unique name + trap. +CANARY_SUFFIX="$$-${RANDOM}" +CANARY="$PLUGIN/.drift-canary.$CANARY_SUFFIX.md" +CANARY2="$PLUGIN/.drift-canary2.$CANARY_SUFFIX.md" +trap 'rm -f "$CANARY" "$CANARY2"' EXIT HUP INT TERM # The canary plants the CANONICAL CAPITALISATION — the form the check must be # able to see. Planting the lowercase form is what let a case-sensitive scan # pass its own control. @@ -112,7 +121,6 @@ require "no prose file states a superseded classification rule" \ bash -c '[ "$0" -eq 0 ] || { printf "%s\n" "$1"; exit 1; }' "$hits" "$report" # ── Rule 3b: positive control for rule 2 ── -CANARY2="$PLUGIN/.drift-canary2.md" printf 'canary: the class is called own-comment here\n' > "$CANARY2" SEEN2=$(grep -rn --include='*.md' -- "own-comment" "$PLUGIN" 2>/dev/null | grep -c 'drift-canary2' || true) rm -f "$CANARY2" @@ -133,5 +141,33 @@ for def in present_re bare_re lead_re; do "def $def:" "$(cat "$SCRIPT")" done +# ── The destructive gate must be EXECUTED, not merely deferred to ── +# +# Pointing at the normative source is what the two readers already did while the +# gate stayed advisory for seven rounds: idd-close described the classification +# faithfully and then judged it itself. Deference is not enforcement. What makes +# it a gate is that the skill runs the helper and obeys its exit code, so that +# is what gets asserted — the invocation, and the fail-closed rule beside it. +CLOSE_MD=$(cat "$PLUGIN/skills/idd-close/SKILL.md") +assert_grep "idd-close resolves the gate helper by path" \ + '/scripts/check-closed-without-summary.sh"' "$CLOSE_MD" +assert_grep "idd-close INVOKES it in single-issue mode" \ + 'bash "$HELPER" --issue "$NUMBER"' "$CLOSE_MD" +assert_grep "idd-close branches on the helper exit code" \ + 'GATE_RC" -ne 0' "$CLOSE_MD" +assert_grep "idd-close states that only exit 0 may proceed" \ + '只有 `rc == 0` 放行' "$CLOSE_MD" +refute_grep "idd-close no longer describes its own gate as prose-only" \ + "本 skill 並未呼叫它" "$CLOSE_MD" + +# ...and the helper must really have the mode the skill invokes. A skill calling +# a flag that does not exist fails open in the worst possible way: `gh`-less +# environments aside, an unknown flag here is warned about and ignored, which +# would put the audit's always-exit-0 contract on the destructive path. +SRC=$(cat "$SCRIPT") +assert_grep "the helper really implements --issue" '--issue) GATE_ISSUE=' "$SRC" +assert_grep "the helper documents the gate exit codes" \ + '0 class == missing, comment set known complete' "$SRC" + print_summary "closing-summary-prose-drift" exit $? diff --git a/plugins/issue-driven-dev/skills/idd-close/SKILL.md b/plugins/issue-driven-dev/skills/idd-close/SKILL.md index 4983aed..8173f4a 100644 --- a/plugins/issue-driven-dev/skills/idd-close/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-close/SKILL.md @@ -58,7 +58,7 @@ allowed-tools: | 正常 `/idd-close` step | `--retroactive` 行為 | |------------------------|----------------------| | Step 0 / 1.5 / 1.6 gates | **跳過**（issue 已關，gate moot；非 force bypass）| -| **Precondition**（retroactive 專屬）| `state == CLOSED` **且**分類為 **`missing`** —— 即**所有 comment 的原始文字裡都找不到** closing-summary heading（分類定義見下方「Precondition 分類」，#295）。OPEN → abort（「不是 retroactive case，跑正常 `/idd-close`」）。**post 前用同一套分類再 check 一次**（防 stale list / race / double-post）。| +| **Precondition**（retroactive 專屬）| **執行** `check-closed-without-summary.sh --issue N` 並依其**退出碼**決定：`0` 才放行，`1`/`2` 一律 abort（分類語意見下方「Precondition 分類」，#295；執行方式與 fail-closed 規則見該節的「這個 gate 必須執行」）。draft 前一次、post 前再一次（防 stale list / race / double-post）。| | Step 2 draft | **reuse，但 `### Verification` section 特別處理** —— 從 `git log --grep "#N"`（Changes）+ 該 issue 既有的 `## Diagnosis` / `## Implementation Complete` / `## Verify` comments + body reconstruct 五段式。**標題改成** `## Closing Summary (retroactive — auto-closed via )`。reconstruct 不足 → 標 **「best-effort reconstruction」**，不假裝完整。**`### Verification` 的捏造風險最高 —— 見下方「Verification honesty 鐵律」。** | | Step 3 confirm | **semi-auto（預設）** —— 把 draft 給 user 確認再 post（reconstruct 可能錯，且 issue 已關不急）。confirm 是必要的、但**不是** verification —— cold-read + batch 容易 rubber-stamp，所以下方鐵律把誠實寫死進 draft，不靠 confirm 兜底。| | Step 4 publish + close | **publish comment，但跳過 `gh issue close`**（已關）。| @@ -98,7 +98,30 @@ allowed-tools: 兩者放行都等於用「補 audit trail」的名義製造重複 audit trail。 -> **本 gate 是散文，不是機械強制（誠實揭露）。** 這張表由 agent 讀了才生效 —— 沒有任何 runtime 會擋住一個忽略它的執行。機械判定只存在於 [`scripts/check-closed-without-summary.sh`](../../scripts/check-closed-without-summary.sh)，而本 skill 並未呼叫它。要把這條路變成真正的 gate，需要 precondition 改為實際執行該 script 並讀其分類 —— **未做**，記在此處而非只留在 PR 討論裡。 +#### 這個 gate 必須**執行**，不是讀完上表自己判（強制，v2.110.0） + +在此之前上表只是散文：沒有任何 runtime 擋得住一個忽略它的執行，機械判定只存在於 helper、而本 skill **並未呼叫它**。也就是說**七輪 verify 的全部成果，要等 agent 剛好讀到那張表才生效**。現在改成真的跑： + +```bash +# draft 之前跑一次；要 post 之前**再跑一次**（防 stale list / race / double-post）。 +# 退出碼就是判決 —— 不要改讀 stdout 的散文再自己決定： +# 0 → class == missing 且 comment 集合完整 → 唯一可以往下走的情況 +# 1 → 其他分類（compliant / casing / present） → abort +# 2 → 無法判定（未 CLOSED / 截斷 / 抓取或解析失敗）→ abort +HELPER="${CLAUDE_PLUGIN_ROOT:-plugins/issue-driven-dev}/scripts/check-closed-without-summary.sh" +[ -f "$HELPER" ] || { echo "✗ 找不到 gate helper：$HELPER —— 中止（找不到 gate 等於沒有 gate）" >&2; exit 1; } + +VERDICT=$(bash "$HELPER" --issue "$NUMBER" ${GITHUB_REPO:+--repo "$GITHUB_REPO"}); GATE_RC=$? +if [ "$GATE_RC" -ne 0 ]; then + echo "✗ /idd-close --retroactive #$NUMBER 中止（rc=$GATE_RC）" >&2 + printf '%s\n' "$VERDICT" | jq -r '" class=\(.class // "?") state=\(.state // "?") comments_complete=\(.comments_complete)\n \(.error // "")"' >&2 + exit 1 +fi +``` + +**只有 `rc == 0` 放行。** `rc != 0` 一律 abort，**包含所有「不確定」的情況**（抓不到、讀不完整、不是 CLOSED、helper 不在）。動作不可逆時 fail-closed 是唯一安全的預設；把不確定讀成「大概沒有 summary 吧」正是會貼出重複內容的那條路。**helper 不在就跳過 gate** 是同一個錯誤的另一種形狀。 + +helper 的 `--issue N` 模式另外做了一件審計模式沒做的事：它用 REST `--paginate` 抓 comment，**不走** `--json comments` 那條硬上限 100 則、且回**最舊** 100 則的路。closing summary 依定義是**最新**一則，所以審計端是事後修補截斷，gate 端是根本不走那條壞路。 **順序固定**：canonical 首行**最先判**，所以 `## Closing Summary (retroactive — …)`（本 skill 自己產出的 heading）落在 `compliant` 而非被 `casing` 分支搶走 —— 那正是 idempotency 依賴的行為。**已知且接受**：`## Closing Summary (draft, do not use)` 同樣讀成 compliant，因此不會被報出來。要擋它就得去界定 heading 尾端，那正是 R5 移除掉的那種解析，而殘留誤差是漏報、不是重複貼文。 From 2b0986d67142dfcb15a8907ea5582cb9d0d043c1 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:34:20 +0800 Subject: [PATCH 04/11] =?UTF-8?q?fix:=20=E5=85=A7=E5=AE=B9=E5=88=A4?= =?UTF-8?q?=E5=AE=9A=E8=88=87=E9=A6=96=E8=A1=8C=E5=88=A4=E5=AE=9A=E5=85=B1?= =?UTF-8?q?=E7=94=A8=E5=90=8C=E4=B8=80=E5=80=8B=E3=80=8C=E8=AE=80=E8=80=85?= =?UTF-8?q?=E7=9C=8B=E5=BE=97=E8=A6=8B=E3=80=8D=E7=9A=84=E5=AE=9A=E7=BE=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lead_line` 找「讀者實際看到的第一行」時會跳過整行 HTML marker（本 plugin 要求 機器可定位的 marker 放第一行，不能讓它把一份逐字正確的 summary 降級）。 `lead_has_content` 掃後續行找內容時**沒有套同一條規則**，於是 HTML 註解被算成 summary。 ## Closing Summary 判成 `compliant`——**印在任何區段都沒有**，而 `--retroactive` 也拒絕它。那正是 內容判定當初要關掉的靜音管道，在下一層又開了一扇門。非 canonical 大小寫的同一 形狀則落 `casing`，那一類是**不帶 ⚠ 印出、並宣稱「summary 在」**。 抽出 `invisible_line`，兩邊共用。對讀者來說空行與看不見的行是同一件事，在這裡 就該是同一件事。 **方向誠實話**：這整類在**便宜**的一側——錯的答案是把 issue 藏起來，不是授權 重複貼文。仍然修，是因為前一輪明講要關掉這個管道、卻只關了一扇。 只認**單行** HTML 註解，這是刻意的：跨行的需要第 1-4 輪移除掉的 fence/comment 狀態機，而殘留誤差同樣落在便宜那一側。 fixture #162 / #163 + 6 條斷言；acid：拿掉後續行的可見性過濾 → 紅 3。 全 suite 47/47，classifier 120 assertions。 --- .../scripts/check-closed-without-summary.sh | 23 +++++++++++++++---- .../fixtures/mixed.json | 20 ++++++++++++++++ .../check-closed-without-summary/test.sh | 21 +++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh b/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh index ad8b2ed..36fa240 100755 --- a/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh +++ b/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh @@ -405,9 +405,23 @@ CLASSIFY=' # markers are skipped, because this plugin mandates a marker on line 1 for # machine-locatable comments (references/dashboard-comment.md), and a leading # marker must not demote a byte-perfect summary. + # ONE notion of "a line a reader sees", used by BOTH the lead test and the + # content test. They used to disagree: lead_line skipped whole-line HTML + # markers, lead_has_content did not, so an HTML comment counted as the summary + # under a heading. `## Closing Summary` + `` read as compliant -- + # printed in NO section at all, and --retroactive refuses it too -- which is + # the silencing channel the content test was added to close, reopened one + # level down. A blank line and an invisible line are the same thing to a + # reader, so they are the same thing here. + # + # Only SINGLE-LINE HTML comments are recognised, deliberately. Spanning ones + # need the fence/comment state machine that rounds 1-4 removed, and the + # residual error is on the cheap side: it hides an issue rather than + # authorising a duplicate post. + def invisible_line: test("^[ \t]*$") or test("^[ \t]*[ \t]*$"); def lead_line: ((. // "") | split("\n")) - | map(select((test("^[ \t]*$") | not) and (test("^[ \t]*[ \t]*$") | not))) + | map(select(invisible_line | not)) | (first // ""); # Oniguruma anchors ^ to the START OF THE STRING, not to each line -- verified, # not assumed. Lines are therefore split explicitly; relying on the anchor @@ -424,8 +438,7 @@ CLASSIFY=' # nothing. def lead_has_content: ((. // "") | split("\n")) as $l - | ([range(0; $l | length) | select(($l[.] | test("^[ \t]*$") | not) - and ($l[.] | test("^[ \t]*[ \t]*$") | not))] | first) as $k + | ([range(0; $l | length) | select($l[.] | invisible_line | not)] | first) as $k | if $k == null then false else # Content is either something non-blank on a LATER line, or something @@ -440,7 +453,9 @@ CLASSIFY=' # silencing channel this check exists to close: anyone who can comment # could post `## Closing Summary` + `...` and remove a closed issue from # the audit permanently. - (($l[($k + 1):] | any(test("[\\p{L}\\p{N}]"))) + # Later lines are filtered through the SAME visibility rule as the lead + # line before being counted -- see `invisible_line`. + (($l[($k + 1):] | map(select(invisible_line | not)) | any(test("[\\p{L}\\p{N}]"))) or ($l[$k] | sub(present_re; ""; "i") | test("[\\p{L}\\p{N}].*[\\p{L}\\p{N}]"))) end; # Four destinations, in order. Only the LAST one authorises anything, and it diff --git a/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/fixtures/mixed.json b/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/fixtures/mixed.json index aecc48d..a3a99f6 100644 --- a/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/fixtures/mixed.json +++ b/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/fixtures/mixed.json @@ -799,5 +799,25 @@ "body": "*Closing Summary*: done\n\n### Problem\nreal content" } ] + }, + { + "number": 162, + "title": "canonical heading whose only content is an HTML comment - invisible in a browser", + "state": "CLOSED", + "comments": [ + { + "body": "## Closing Summary\n" + } + ] + }, + { + "number": 163, + "title": "non-canonical heading whose only content is an HTML comment", + "state": "CLOSED", + "comments": [ + { + "body": "## closing summary\n" + } + ] } ] diff --git a/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/test.sh b/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/test.sh index 10c6156..2605e88 100644 --- a/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/test.sh +++ b/plugins/issue-driven-dev/scripts/tests/check-closed-without-summary/test.sh @@ -387,6 +387,27 @@ require "prose mentioning the marker still cannot rescue an issue" \ bash -c 'printf "%s" "[{\"number\":9001,\"title\":\"p\",\"state\":\"CLOSED\",\"comments\":[{\"body\":\"I forgot the closing summary, sorry\"}]}]" > "$0/p.json"; bash "$1" --json-file "$0/p.json" | grep -q "9001"' "${TMPDIR:-/tmp}" "$HELPER" +# ── the content test must use the same notion of "visible" as the lead test ── +# +# `lead_line` skips whole-line HTML markers when deciding which line a reader +# sees first — this plugin mandates a machine-locatable marker on line 1, and it +# must not demote a byte-perfect summary. `lead_has_content` did NOT apply the +# same rule to the lines it scans for content, so an HTML comment counted as a +# summary. That reopens, one level down, the silencing channel the content test +# was added to close: `## Closing Summary` + `` read as compliant, +# which prints in NO section at all, and `--retroactive` refuses it too. +# +# Direction note: this whole class sits on the CHEAP side — the wrong answer +# hides an issue rather than authorising a duplicate post. It is fixed anyway +# because a previous round set out to close exactly this channel and left a +# second door open. +require "#162 (canonical heading, only an HTML comment under it) is PRESENT" unverified 162 +refute "#162 is NOT compliant — compliant prints nowhere, i.e. invisible" in_section "CASING —" 162 +refute "#162 is NOT in MISSING" flagged 162 +require "#163 (casing heading, only an HTML comment under it) is PRESENT" unverified 163 +refute "#163 is NOT in CASING — CASING claims the summary is there" in_section "CASING —" 163 +refute "#163 is NOT in MISSING" flagged 163 + # ── `--issue N`: the single-issue GATE (#307 follow-up) ──────────────────────── # Audit mode reports to a human and always exits 0. This mode is a precondition # for an IRREVERSIBLE action, so the whole point is the exit code: the caller From feec9384e4001738778a22c1cdd1efe4e2e0059e Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:40:30 +0800 Subject: [PATCH 05/11] =?UTF-8?q?fix:=20=E5=AF=AC=E9=AC=86=E6=AF=94?= =?UTF-8?q?=E5=B0=8D=E4=B8=8D=E5=BE=97=E7=94=A8=E4=BE=86=E5=81=9A=E6=AD=A3?= =?UTF-8?q?=E9=9D=A2=E6=96=B7=E8=A8=80=EF=BC=88idd-find=20/=20idd-update?= =?UTF-8?q?=EF=BC=89=EF=BC=8C=E9=99=84=E4=BB=B6=20URL=20=E4=B8=89=E7=A8=AE?= =?UTF-8?q?=E5=8C=85=E8=A3=B9=E5=BD=A2=E5=BC=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit normative source 把 predicate 拆成兩個，判準不是「哪個比較準」而是**過度偵測 會往哪個方向錯**：`present_re`（寬）回答「讀者看得到嗎」，多報一次只是不採取 破壞性動作；`lead_re`（嚴）回答「這則 comment 是不是以它開頭」，多報一次就是 講了一句假話。這條規則不只屬於 classifier，但有兩個散文 reader 站錯邊： - **`idd-find`** 用寬鬆比對標 `📜 closing summary（可考古的結案紀錄）`。一則 只是引用模板來提問的 comment 會拿到一模一樣的標籤。改成 `📜 summary marker` ——**標籤的字面就是它能保證的全部**。比對維持寬鬆（搜尋結果漏報的誤導成本 更高），改的是它敢宣稱什麼。render 範例那行一起改：只改被點到的那行、留下 隔壁那行，正是這個 marker 五輪漂移的原樣。 - **`idd-update`** 的 phase 推斷同樣用寬鬆比對，而 phase 是正面斷言（「這個 階段發生過」）。一則引用 `> ## Closing Summary` 的討論就能把一張**還開著**的 issue 推成 `closed`。改成要求該 heading 是那則 comment 的首行、不帶 blockquote 前綴、縮排 ≤3 空格 —— 即 `lead_re`。 **附件 URL 抽取**：字元類只排除 `)` 與空白，也就是只涵蓋 markdown link。實測 另外三種真實 issue body 的包裹方式全部把包裹字元黏了回來： autolink → 尾巴多 `>` HTML 屬性 → 尾巴多 `">` see https://…/c.pdf. 句尾 → 尾巴多 `.` 三種都會 404，而抓不下來的附件就是被忽略的來源 —— 本 plugin 把那當成違規而非 不便。`<` `>` `"` `'` 不可能未編碼地出現在 URL 裡，排除掉是零成本；句尾標點改 成事後去掉一個（`.` 在 URL 中段合法，而這些網址都以副檔名結尾）。 **jq 錯誤路徑**：註解寫著「strip control characters and bidi overrides」，程式 只做前者 —— `tr` 在 LC_ALL=C 下處理**位元組**，看不到多位元組的 U+202E / U+2028。 先量了這個縫**通不通**：三種 payload（U+202E / U+2028 / ESC）放在會讓 `.state` 撞上裸字串的位置，jq 1.7 回 `Cannot index string with string ("state")`，**完全 不回顯輸入值**。所以改的是註解不是程式碼，並如實記下這個乾淨的否定結果 —— 補一份 Unicode-aware scrub 等於多一份字元類副本，而「安全定義的副本各自漂移」 正是這個檔案最長的病史。哪天遇到會回顯的 jq，要改的就是那一行。 新增：process-attachments 的 `wrapped` gh-stub mode + 5 條斷言（RED 3 條）； prose-drift 補 3 條斷言釘住兩個 reader 的方向。全 suite 47/47。 --- .../scripts/check-closed-without-summary.sh | 22 ++++++++++++++--- .../scripts/process-attachments.sh | 20 +++++++++++++++- .../tests/closing-summary-prose-drift/test.sh | 18 ++++++++++++++ .../scripts/tests/process-attachments/test.sh | 24 +++++++++++++++++++ .../issue-driven-dev/skills/idd-find/SKILL.md | 7 +++--- .../skills/idd-update/SKILL.md | 8 +++++-- 6 files changed, 90 insertions(+), 9 deletions(-) diff --git a/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh b/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh index 36fa240..0745725 100755 --- a/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh +++ b/plugins/issue-driven-dev/scripts/check-closed-without-summary.sh @@ -495,9 +495,25 @@ CLASSIFY=' JQ_ERR=$(mktemp "${TMPDIR:-/tmp}/csw_jq_err.XXXXXX") || JQ_ERR="" if ! CLASSIFIED=$(printf '%s' "$ISSUES_JSON" | jq -r "$CLASSIFY" 2>"${JQ_ERR:-/dev/null}"); then echo "note: classification filter failed — audit skipped, no conclusion drawn." >&2 - # jq quotes the offending INPUT in its message, so this text is untrusted: - # control characters and bidi overrides here would bypass `sanitize` entirely - # and repaint the terminal. Strip them, and cap the volume. + # jq may quote the offending INPUT in its message, so treat this text as + # untrusted and cap the volume. + # + # WHAT THIS ACTUALLY STRIPS, stated precisely because the comment used to + # claim more than the code did: ASCII control characters, and nothing else. + # `tr` under LC_ALL=C works on BYTES, so it cannot see U+202E, U+2028 or the + # bidi isolates — they are multi-byte, and deleting their bytes individually + # would corrupt the surrounding UTF-8. Only `sanitize` (inside CLASSIFY) has + # the Unicode-aware version, and this path is precisely the one where CLASSIFY + # did not run. + # + # Whether that gap is reachable was measured, not assumed: three payloads + # (U+202E, U+2028, ESC) planted where a bare string reaches `.state` produce + # `Cannot index string with string ("state")` on jq 1.7 — the value is NOT + # echoed, so nothing attacker-controlled arrives here at all. Recorded as a + # clean negative rather than fixed: a Unicode-aware scrub would mean a second + # copy of the character class, and a divergent copy of a safety definition is + # the failure mode this file has the longest history with. If a jq that does + # echo the value ever shows up, this is the line to change. [ -n "$JQ_ERR" ] && LC_ALL=C tr -d "\000-\010\013\014\016-\037\177" < "$JQ_ERR" \ | head -5 | sed "s/^/ jq: /" >&2 [ -n "$JQ_ERR" ] && rm -f "$JQ_ERR" diff --git a/plugins/issue-driven-dev/scripts/process-attachments.sh b/plugins/issue-driven-dev/scripts/process-attachments.sh index d0d191e..df699ab 100755 --- a/plugins/issue-driven-dev/scripts/process-attachments.sh +++ b/plugins/issue-driven-dev/scripts/process-attachments.sh @@ -127,8 +127,26 @@ detect_urls() { local raw content raw=$(gh issue view "$NUMBER" --repo "$REPO" --json body,comments) || return 2 content=$(printf '%s\n' "$raw" | jq -r '.body, .comments[].body') || return 2 + # The character class excluded `)` and whitespace only, which covers a + # markdown link and nothing else. Real issue bodies wrap URLs three more ways, + # and each one used to come back with the wrapper glued on: + # + # autolink -> trailing `>` + # HTML attribute -> trailing `">` + # see https://…/c.pdf. end of sentence -> trailing `.` + # + # All three download as 404, and an attachment that cannot be downloaded is a + # source that gets ignored — which this plugin treats as a rule violation, not + # a nuisance. `<`, `>`, `"` and `'` can never appear unencoded in a URL, so + # excluding them is free. + local u='[^)<>"'"'"'[:space:]]' + # Trailing sentence punctuation is stripped afterwards rather than excluded, + # because `.` is legal mid-URL and every one of these ends in a file + # extension. Exactly one character, so `…/c.pdf..` would still keep a dot — + # accepted: the wrong direction here is a loud 404, not a silent wrong file. printf '%s\n' "$content" \ - | grep -oE 'https://(github\.com/(user-attachments/(files|assets)/[^)[:space:]]+|[^/]+/[^/]+/files/[0-9]+/[^)[:space:]]+|[^/]+/[^/]+/releases/download/[^/)[:space:]]+/[^)[:space:]]+)|(private-)?user-images\.githubusercontent\.com/[^)[:space:]]+)' \ + | grep -oE "https://(github\.com/(user-attachments/(files|assets)/$u+|[^/]+/[^/]+/files/[0-9]+/$u+|[^/]+/[^/]+/releases/download/[^/)<>\"'[:space:]]+/$u+)|(private-)?user-images\.githubusercontent\.com/$u+)" \ + | sed 's/[.,;:!?]$//' \ | sort -u || true } diff --git a/plugins/issue-driven-dev/scripts/tests/closing-summary-prose-drift/test.sh b/plugins/issue-driven-dev/scripts/tests/closing-summary-prose-drift/test.sh index f7f46af..363c648 100644 --- a/plugins/issue-driven-dev/scripts/tests/closing-summary-prose-drift/test.sh +++ b/plugins/issue-driven-dev/scripts/tests/closing-summary-prose-drift/test.sh @@ -160,6 +160,24 @@ assert_grep "idd-close states that only exit 0 may proceed" \ refute_grep "idd-close no longer describes its own gate as prose-only" \ "本 skill 並未呼叫它" "$CLOSE_MD" +# ── Permissive matching may not back a POSITIVE claim ── +# +# The two-predicate split is not local to the classifier: it is a rule about +# which question is being asked. `present_re` (permissive) answers "could a +# reader see one?", where over-detecting is safe. `lead_re` (strict) answers +# "does this comment lead with one?", where over-detecting states something +# false. Two prose readers were on the wrong side of it — idd-find labelled a +# quotation as an archaeological record, and idd-update pushed an OPEN issue's +# phase to `closed` on a quoted heading. +UPDATE_MD=$(cat "$PLUGIN/skills/idd-update/SKILL.md") +FIND_MD=$(cat "$PLUGIN/skills/idd-find/SKILL.md") +assert_grep "idd-update requires the heading to LEAD the comment (phase is a positive claim)" \ + "必須是那則 comment 的首行" "$UPDATE_MD" +refute_grep "idd-update no longer allows a blockquote prefix for phase inference" \ + "允許任意縮排與 blockquote 前綴" "$UPDATE_MD" +refute_grep "idd-find no longer calls a permissive match an archaeological record" \ + "標 \`📜 closing summary\`（可考古的結案紀錄）" "$FIND_MD" + # ...and the helper must really have the mode the skill invokes. A skill calling # a flag that does not exist fails open in the worst possible way: `gh`-less # environments aside, an unknown flag here is warned about and ignored, which diff --git a/plugins/issue-driven-dev/scripts/tests/process-attachments/test.sh b/plugins/issue-driven-dev/scripts/tests/process-attachments/test.sh index 46347f6..cd71bb1 100755 --- a/plugins/issue-driven-dev/scripts/tests/process-attachments/test.sh +++ b/plugins/issue-driven-dev/scripts/tests/process-attachments/test.sh @@ -28,6 +28,11 @@ case "${1:-}" in case "${GH_STUB_MODE:-empty}" in empty) printf '{"body":"## Problem\\nno attachment urls here","comments":[{"body":"plain comment"}]}\n' ;; with_url) printf '{"body":"spec: https://github.com/user-attachments/files/123/spec.docx ok","comments":[]}\n' ;; + # The three ways a real issue body wraps a URL. Each used to come back + # with the wrapper glued on, producing a link that 404s — and an + # attachment that cannot be downloaded is an attachment that gets + # ignored, which this plugin treats as ignoring the source. + wrapped) printf '{"body":"autolink \\nhtml \\nsentence see https://github.com/user-attachments/files/3/c.pdf.","comments":[]}\n' ;; fail) echo "gh: network error (stub)" >&2; exit 1 ;; esac ;; auth) echo "stub-token" ;; @@ -134,5 +139,24 @@ refute "f10a schemaless manifest ({\"foo\":1}) → check exits non-zero" test require "f10b schemaless manifest → check says corrupt/malformed (loud)" grep -qiE 'corrupt|malformed' "$W/out10.txt" cd /; rm -rf "$W" +# ── Fixture 11 (post-merge audit): URLs wrapped the way real issue bodies wrap +# them. The extractor's character class excluded `)` and whitespace only, so an +# autolink kept its `>`, an HTML attribute kept its `">`, and a URL at the end +# of a sentence kept the full stop. Each of those downloads 404s, and a file +# that cannot be downloaded is a source that gets ignored — the one thing the +# attachment rule says must never happen quietly. +W="$(mktemp -d)"; cd "$W" +export GH_STUB_MODE=wrapped +run_pa download 21 > "$W/out11.txt" 2>&1 +MAN11=".claude/.idd/attachments/issue-21/_manifest.json" +require "f11a wrapped-URL run still writes a manifest" test -f "$MAN11" +URLS11=$(jq -r '.files[].url, (.errors[]?.url // empty)' "$MAN11" 2>/dev/null; jq -r '.[]?.url // empty' "$MAN11" 2>/dev/null) +# Whatever the manifest ends up recording, no recorded URL may carry a wrapper. +refute_grep_re "f11b no extracted URL keeps an autolink '>'" '>' "$URLS11" +refute_grep_re "f11c no extracted URL keeps an HTML quote" '"' "$URLS11" +refute_grep_re "f11d no extracted URL keeps a sentence full stop" '\.$' "$URLS11" +assert_eq "f11e all three URLs were extracted" "3" "$(printf '%s\n' "$URLS11" | grep -c 'github.com')" +cd /; rm -rf "$W" + rm -rf "$STUB" print_summary diff --git a/plugins/issue-driven-dev/skills/idd-find/SKILL.md b/plugins/issue-driven-dev/skills/idd-find/SKILL.md index a33c571..a656809 100644 --- a/plugins/issue-driven-dev/skills/idd-find/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-find/SKILL.md @@ -80,15 +80,16 @@ fallback 啟用時印一行 notice（排序品質可能較差）。 ### Step 3: IDD overlay - **Open hit**：`gh issue view N --json body` 解析 `**Phase**:` 行（同 idd-list Step 3 規則；無 → `(no phase)`）；`gh pr list --state open` body scan `#N` → 有 → 標 `→ PR #M`（精簡版 — 不做 cluster leader 邏輯，要完整視圖導流 idd-list） -- **Closed hit**：comments 掃 `## Closing Summary`（**出現在任何一行即可，不限於 comment 開頭** —— #295 實測的兩種漂移是「大小寫」與「summary 併進 Implementation Complete 那一則」，只認開頭會把後者標成 `(closed, no summary)`），**大小寫不敏感、允許任意縮排與 blockquote 前綴、1-6 個井號、井號與字之間的裝飾字元（emoji）**（#295 —— 該 heading 由 LLM 依模板生成、寫入端無 normalization，形式漂移是預期而非例外）→ 有 → 標 `📜 closing summary`（可考古的結案紀錄）；無 → 標 `(closed, no summary)` - - 本標記只回答「有沒有可考古的紀錄」，**不做**上述分類 —— 那是 `--audit-closes` 與 `--retroactive` 的判定（normative source：`scripts/check-closed-without-summary.sh`）。但大小寫敏感會把 `## Closing summary` 標成 `(closed, no summary)`、只認 comment 開頭會把「併進 IC 那一則」標成同樣的話，對搜尋結果都是誤導，故此處採寬鬆比對 +- **Closed hit**：comments 掃 `## Closing Summary`（**出現在任何一行即可，不限於 comment 開頭** —— #295 實測的兩種漂移是「大小寫」與「summary 併進 Implementation Complete 那一則」，只認開頭會把後者標成 `(closed, no marker)`），**大小寫不敏感、允許任意縮排與 blockquote 前綴、1-6 個井號、井號與字之間的裝飾字元（emoji）**（#295 —— 該 heading 由 LLM 依模板生成、寫入端無 normalization，形式漂移是預期而非例外）→ 有 → 標 `📜 summary marker`；無 → 標 `(closed, no marker)` + - **標籤的字面就是它能保證的全部：那個 marker 出現過。** 比對刻意寬鬆（引述、fence 內、blockquote 前綴一律算「有」），所以它**不能**宣稱「有一份可考古的結案紀錄」—— 一則只是引用模板來提問的 comment 會拿到一模一樣的標籤。舊標籤寫 `📜 closing summary（可考古的結案紀錄）`，那是**用寬鬆比對做正面斷言**，正是 normative source 要分成 `present_re`（寬，問「有沒有」）與 `lead_re`（嚴，問「是不是」）所要避免的事 + - 本標記**不做**四分類 —— 那是 `--audit-closes` 與 `--retroactive` 的判定（normative source：`scripts/check-closed-without-summary.sh`）。這裡仍用寬鬆比對，理由是搜尋結果的誤導成本：大小寫敏感會把 `## Closing summary` 標成沒有、只認 comment 開頭會把「併進 IC 那一則」標成沒有 ### Step 4: Render ``` Find: "comment surgery escape" (repo: owner/repo, 15 hits max) - 1. #150 [closed 📜 closing summary] fix: idd-edit body wipe on bad sed — updated 2mo ago + 1. #150 [closed 📜 summary marker] fix: idd-edit body wipe on bad sed — updated 2mo ago 2. #158 [implemented → PR #257] idd-edit batch × R5 refuse semantics — updated 4d ago 3. ... diff --git a/plugins/issue-driven-dev/skills/idd-update/SKILL.md b/plugins/issue-driven-dev/skills/idd-update/SKILL.md index 6bb9881..5ca1ae9 100644 --- a/plugins/issue-driven-dev/skills/idd-update/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-update/SKILL.md @@ -119,9 +119,13 @@ gh issue view $NUMBER --repo $GITHUB_REPO --json title,body,labels,state,comment | Verify (FAIL / findings) | `needs-fix` | | Closing Summary | `closed` | -判斷依據：掃描 comments 中的 `## Diagnosis`、`## Implementation Plan`、`## Implementation Complete`、`## Verify`、`## Closing Summary` 標題。**比對大小寫不敏感、允許任意縮排與 blockquote 前綴、1-6 個井號、井號與字之間的裝飾字元（emoji）**（#295）—— 這些 heading 全部由 LLM 依模板生成、寫入端沒有任何 normalization，所以 `## Closing summary` 這類漂移是預期而非例外；此處硬要求大小寫只會讓 phase 停在舊值，而 phase 停在舊值正是 `idd-close` Step 6 存在的理由。 +判斷依據：掃描 comments 中的 `## Diagnosis`、`## Implementation Plan`、`## Implementation Complete`、`## Verify`、`## Closing Summary` 標題。**比對大小寫不敏感、1-6 個井號、井號與字之間的裝飾字元（emoji）**（#295）—— 這些 heading 全部由 LLM 依模板生成、寫入端沒有任何 normalization，所以 `## Closing summary` 這類漂移是預期而非例外；此處硬要求大小寫只會讓 phase 停在舊值，而 phase 停在舊值正是 `idd-close` Step 6 存在的理由。 -> **本步是這個 marker 的寫端 reader（#295 family-wide scope 的第 6 個）**。它**不做**分類分流 —— phase 推斷只需要「有沒有」，不需要「是哪一種」。分類的 normative source 是 [`scripts/check-closed-without-summary.sh`](../../scripts/check-closed-without-summary.sh)，消費者是 `--audit-closes` 與 `--retroactive`。 +**但該 heading 必須是那則 comment 的首行**（跳過空行與整行 HTML marker 後的第一行），**且不得帶 blockquote 前綴、縮排不超過 3 空格** —— 也就是 normative source 的 `lead_re`，不是 `present_re`。 + +> **為什麼這一步用嚴的那個 predicate**：phase 是一句**正面斷言**（「這個階段發生過」）。用寬鬆比對的話，一則只是**引用**模板來討論的 comment（`> ## Closing Summary`，或貼在 fence 裡的範例）就會把一張還開著的 issue 的 phase 推到 `closed`。normative source 之所以拆成兩個 predicate，判準正是**過度偵測會往哪個方向錯**：問「有沒有」時寬鬆是安全的（多報一次只是不採取破壞性動作），做正面斷言時寬鬆就是造假。`idd-find` 的 `📜 summary marker` 是前者，所以它維持寬鬆、而且標籤只敢說「marker 出現過」。 + +> **本步是這個 marker 的寫端 reader（#295 family-wide scope 的第 6 個）**。它**不做**四分類分流 —— phase 推斷只問「這則 comment 是不是以它開頭」。分類的 normative source 是 [`scripts/check-closed-without-summary.sh`](../../scripts/check-closed-without-summary.sh)，消費者是 `--audit-closes` 與 `--retroactive`。 #### Authoritative source resolution (v2.73.0+, #150) From 520191922b58408eb2a25d14fd12944990019ead Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:45:46 +0800 Subject: [PATCH 06/11] =?UTF-8?q?fix:=20idd-verify=20=E7=9A=84=E6=9A=AB?= =?UTF-8?q?=E5=AD=98=E8=B7=AF=E5=BE=91=E7=9C=9F=E7=9A=84=E6=94=B9=E6=8E=89?= =?UTF-8?q?=E4=BA=86=EF=BC=88#288=20=E7=9A=84=2024=20=E8=99=95=E6=AE=98?= =?UTF-8?q?=E7=95=99=EF=BC=89=EF=BC=8C=E4=B8=A6=E7=94=A8=E6=B8=AC=E8=A9=A6?= =?UTF-8?q?=E9=87=98=E4=BD=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #288 的 closing summary 說「每個 scratch path 現在都掛在 `mktemp -d` 底下」。 實際出貨的是一段**這樣寫的註記**，後面接著二十四處繼續 MUST 舊的固定路徑 —— 包含交給 reviewer agent 的 OUTPUT 指示，也就是真正決定 findings 落在哪裡的那 幾行。規則和違反它的例子相隔四行，共存了兩個版本。 **一份先寫規則、隔一行又自打嘴巴的規格，比什麼都沒寫更糟**：去查規則的人查得 到，照著抄命令的人抄到缺陷。 固定名稱不帶 repo 身分：同一個 issue 號在不同 repo 的兩個 session 共用檔名， 前一輪的殘檔會被當成這一輪的 findings 讀進來 —— 靜默，且方向最壞（把別的 repo 的判決併進這份報告）。 全部改掛 `$VERIFY_DIR`（Step 0 新增 `resolve_scratch_dir` task，在任何寫檔或 spawn 之前解析一次）。過程中另外撞到兩個**同一段迴圈的兩份副本**——egress body （`master.md` / `pointer.md` / `pointer_template.md`）也用固定名，而那是要被 **貼到別人 issue 上的文字**：共用路徑上的殘檔或半寫檔不會大聲失敗，它會發布錯的 留言。兩份都改了；第二份是機械掃描抓到的，不是我看到的。 新增 `scripts/tests/verify-scratch-paths/`（第 48 個 suite）：掃固定 /tmp 路徑， 帶 positive control（唯一檔名 + trap）。 **scope 明講**：只掃 `idd-verify`。同一個 grep 也會打到 `idd-edit` 的 `/tmp/idd-edit-backup/` 與 `/tmp/idd-edit-repl-*` —— 那是另一回事、**未涵蓋**： backup 目錄是文件叫使用者去 `ls` 的復原位置，搬它是行為變更，且碰撞後果是看得見 的衝突而非被靜默併入的判決。這條寫進測試檔本身，免得有人把它的綠讀成對 idd-edit 的背書。 註記本身也不再逐字寫出被禁的舊路徑 —— 同 prose-drift 測試禁止複述 classifier regex 的理由：不存在的副本不會漂移，而寫進文件的違例字面正是機械檢查第一個踩到 的東西。 全 suite 48/48。 --- .../tests/verify-scratch-paths/test.sh | 72 ++++++++++ .../skills/idd-verify/SKILL.md | 128 ++++++++++-------- 2 files changed, 144 insertions(+), 56 deletions(-) create mode 100755 plugins/issue-driven-dev/scripts/tests/verify-scratch-paths/test.sh diff --git a/plugins/issue-driven-dev/scripts/tests/verify-scratch-paths/test.sh b/plugins/issue-driven-dev/scripts/tests/verify-scratch-paths/test.sh new file mode 100755 index 0000000..ed76e88 --- /dev/null +++ b/plugins/issue-driven-dev/scripts/tests/verify-scratch-paths/test.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# Test: no skill may name a FIXED scratch path under /tmp (#288). +# +# WHY THIS EXISTS +# +# #288 was closed with a summary stating that every scratch path now hung off +# `mktemp -d`. What actually shipped was a NOTE saying so, followed by twenty- +# three paragraphs that went on MUSTing `/tmp/verify_${NUMBER}_*` — including +# the OUTPUT instructions handed to the reviewer agents, i.e. the paths that +# decide where findings actually land. The rule and its violations lived four +# lines apart for two releases. +# +# A fixed name under /tmp carries no repo identity. Two sessions verifying the +# SAME issue number in DIFFERENT repos share filenames, and a leftover file from +# the earlier run is read as this run's findings — silently, and in the worst +# direction: another repo's verdict merged into this PR's report. +# +# So the rule is mechanised rather than restated. Prose cannot drift from a +# grep. +set -u +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PLUGIN="$(cd "$HERE/../../.." && pwd)" +. "$(cd "$HERE/../../lib" && pwd)/assert-helpers.sh" + +# A fixed scratch path = /tmp (or ${TMPDIR:-/tmp}) followed by a literal name. +# `mktemp` lines are exempt: that is the sanctioned way to obtain one, and the +# template it takes necessarily contains /tmp. +# +# SCOPE, stated rather than implied: this scans `idd-verify` only. The same +# grep over all of skills/ also hits `idd-edit`, which writes +# `/tmp/idd-edit-backup/` and `/tmp/idd-edit-repl-${COMMENT_ID}.md`. Those are a +# different problem and are NOT covered here: the backup directory is a +# documented recovery location users are told to `ls`, so moving it is a +# behaviour change, and the collision consequence there is a visible clash +# rather than a silently merged verdict. Filed separately — do not read this +# file's green as a statement about idd-edit. +scan_fixed_tmp() { + grep -rnE --include='*.md' -- '(^|[^A-Za-z0-9_])/tmp/[A-Za-z0-9_.-]' \ + "$PLUGIN/skills/idd-verify" 2>/dev/null \ + | grep -v 'mktemp' \ + | grep -v 'TMPDIR:-/tmp' +} + +HITS=$(scan_fixed_tmp || true) +require "no skill names a fixed scratch path under /tmp" \ + bash -c '[ -z "$0" ] || { printf "%s\n" "$0"; exit 1; }' "$HITS" + +# Positive control. Without it, a scan broken by a bad flag or a wrong path +# (all of which have happened in this repo) reads as a clean tree. Unique name +# + trap, because the canary is written INTO the repo and an interrupted run +# would otherwise leave it there as a permanent red. +CANARY="$PLUGIN/skills/idd-verify/.tmp-path-canary.$$-${RANDOM}.md" +trap 'rm -f "$CANARY"' EXIT HUP INT TERM +printf 'canary: write findings to /tmp/verify_${NUMBER}_findings_logic.md\n' > "$CANARY" +SEEN=$(scan_fixed_tmp | grep -c 'tmp-path-canary' || true) +rm -f "$CANARY" +require "positive control: the scan actually detects a planted fixed path" \ + bash -c '[ "$0" -ge 1 ]' "$SEEN" + +# The sanctioned replacement must be present and resolved BEFORE anything is +# written — a run directory created after the first write is not a run +# directory, it is a rename. +VERIFY_MD=$(cat "$PLUGIN/skills/idd-verify/SKILL.md") +assert_grep "idd-verify resolves a per-run scratch dir with mktemp -d" \ + 'VERIFY_DIR=$(mktemp -d' "$VERIFY_MD" +assert_grep "...as a Step 0 task, before any spawn or write" \ + 'TaskCreate(name="resolve_scratch_dir"' "$VERIFY_MD" +assert_grep "reviewer OUTPUT instructions use it" \ + '$VERIFY_DIR/findings_' "$VERIFY_MD" + +print_summary "verify-scratch-paths" +exit $? diff --git a/plugins/issue-driven-dev/skills/idd-verify/SKILL.md b/plugins/issue-driven-dev/skills/idd-verify/SKILL.md index 68a1b96..74427f9 100644 --- a/plugins/issue-driven-dev/skills/idd-verify/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-verify/SKILL.md @@ -286,14 +286,15 @@ PAI_ENGINE="${PAI_DIR}workflows/ensemble-workflow.js" TaskCreate(name="resolve_input_source", description="Step 0.5: 解析 --pr / --commits / --branch / --since flag；都沒帶就跑 auto-detect（count Refs #N commits since origin/，再 gh pr list 找 open PR），有歧義時 AskUserQuestion 確認") TaskCreate(name="gate_pr_correspondence", description="Step 0.7: PR mode 下強制檢查 issue↔PR 對應 — gh pr view --json body 抓 Refs #N，跟 user 指定的 issue 比對；PR 沒任何 Refs 或 user issue 不在 set 內 → abort 並告訴使用者怎麼修") TaskCreate(name="scan_pr_body_and_commits_trailers", description="Step 0.8: PR mode 下兩 source 偵測 auto-close trap — (1) gh pr view --json closingIssuesReferences 查 PR body 是否 linked-to-auto-close（GitHub 權威解析、所有 trailer 形式），(2) gh pr view --json commits 對每個 commit messageBody 跑 trap regex（補上 GitHub 不預計算的 commit-body channel — squash 後字串 land 在 main 觸發 auto-close）。任一非空則 warn — bypass /idd-close gate。Warn-only，不 abort") -TaskCreate(name="get_diff_and_issue", description="依 input source 取 diff（gh pr diff / git diff HEAD~N / git diff origin/...） + gh issue view,存 diff 到 /tmp 供 agents 讀取,並記 FROZEN_SHA=$(git rev-parse HEAD)（PR mode 記 PR head oid — #228 freshness 錨點）；PR mode 額外做 gh pr checkout 並記住原 branch") +TaskCreate(name="resolve_scratch_dir", description="Step 0.4 (#288): VERIFY_DIR=$(mktemp -d \"${TMPDIR:-/tmp}/idd-verify-${NUMBER}-XXXXXX\") — 一次解析、之後所有 diff / prompt / findings / codex 檔全部掛在它底下。**必須在任何寫檔或 spawn 之前**。固定名稱（舊的 /tmp/verify_${NUMBER}_*）不帶 repo 身分，同一個 issue 號在不同 repo 的兩個 session 會共用檔名，前一輪的殘檔會被當成這一輪的 findings 讀進來 —— 靜默，且方向最壞（把別的 repo 的判決併進這份報告）") +TaskCreate(name="get_diff_and_issue", description="依 input source 取 diff（gh pr diff / git diff HEAD~N / git diff origin/...） + gh issue view,存 diff 到 $VERIFY_DIR/diff.patch 供 agents 讀取,並記 FROZEN_SHA=$(git rev-parse HEAD)（PR mode 記 PR head oid — #228 freshness 錨點）；PR mode 額外做 gh pr checkout 並記住原 branch") TaskCreate(name="check_attachments", description="確認 .claude/.idd/attachments/issue-NNN/ 存在,把 attachment 路徑塞進 reviewer agent prompt 作為 source-of-truth context。manifest 缺漏 → 警告繼續(reviewer 仍跑,但 verification 完整度受限)。依 rules/process-attachments.md。") TaskCreate(name="resolve_dispatch_model", description="解析 $AGENT_MODEL — IDD_AGENT_MODEL 未設 → opus；非法值 → abort with usage error（#205；兩個 backend 共用，Workflow args 傳 agentModel、manual 模板填 model）；#264 同步解析 codex 治理（check-plugin-presence.sh codex-pro codex-pro → CP defaults.json + profile.yaml 兩層 → CODEX_MODEL/EFFORT/MAX_TIME，缺席 fail-fast）") TaskCreate(name="launch_parallel_reviewers", description="第一波 5 個 tool calls 同一 message: 4 lens Agent(subagent_type=general-purpose, model=$AGENT_MODEL) for requirements/logic/security/regression + 1 Bash codex(run_in_background:true)；DA 不在此波（#130 sequenced）。prompt 引用 attachment 路徑 + 強制 file-output rule (per #52)") TaskCreate(name="spawn_sequenced_da", description="#130: 4 份 lens findings 檔全部就緒（non-empty）後，coordinator 序列 spawn Devil's Advocate（model=$AGENT_MODEL，prompt 直附 4 檔路徑，無 polling）") -TaskCreate(name="wait_for_claude_agents", description="4 lens Agent calls return 後 ls /tmp/verify_${NUMBER}_findings_*.md 確認 4 檔 non-empty（DA 檔在 sequenced spawn 後另計）;缺者進 Step 2.5 Recovery Protocol") +TaskCreate(name="wait_for_claude_agents", description="4 lens Agent calls return 後 ls $VERIFY_DIR/findings_*.md 確認 4 檔 non-empty（DA 檔在 sequenced spawn 後另計）;缺者進 Step 2.5 Recovery Protocol") TaskCreate(name="recovery_protocol", description="Step 2.5 (NEW per #52): 缺 findings 檔者 SendMessage retry with FULL context re-paste(不假設 context 倖存 idle/wake);二次 idle → coordinator self-review for that role + 在 master report 標 process gap") -TaskCreate(name="wait_for_codex", description="等 Codex 背景任務完成,讀 /tmp/codex-verify-${NUMBER}.md") +TaskCreate(name="wait_for_codex", description="等 Codex 背景任務完成,讀 $VERIFY_DIR/codex.md") TaskCreate(name="freshness_gate", description="Step 2.9 (#228): merge/aggregate 前比對 FROZEN_SHA vs 當前 HEAD（PR mode: PR head oid）— 不一致 → 拒絕 aggregate,要求 re-freeze + 補審 delta round;一致才放行 merge") TaskCreate(name="merge_findings", description="合併 6 個來源 findings 去重,severity 取最高") TaskCreate(name="post_master_and_pointers", description="PR mode: master 貼到 PR + capture URL → 為每個 ref'd issue 貼 pointer comment；本地 mode: 貼到 issue（單 issue 直接貼／多 issue 用 SOP master+pointer）") @@ -308,7 +309,7 @@ TaskCreate(name="triage_followup_issues", description="Step 5b: 分類 non-block **v2.32.0+ tagging 規則**：若 Verify findings comment 要 @-tag 寫 code 的人或要求審閱者，**必須**遵循 [`rules/tagging-collaborators.md`](../../rules/tagging-collaborators.md) 5 步協定（gh api → fuzzy match → AskUserQuestion fallback → @login 不用 display name → post 前 verify）。違反 = 通知錯人，不可逆。 **鐵律**: -- `wait_for_claude_agents` 和 `wait_for_codex` 都要跑到真的有 findings 內容,不能只看到 Agent return / idle notification 就 completed — 必須 `ls /tmp/verify_${NUMBER}_findings_*.md` 確認 5 個檔案 + non-empty +- `wait_for_claude_agents` 和 `wait_for_codex` 都要跑到真的有 findings 內容,不能只看到 Agent return / idle notification 就 completed — 必須 `ls $VERIFY_DIR/findings_*.md` 確認 5 個檔案 + non-empty - 如果某個 reviewer 沒寫 findings 檔 → 進 Step 2.5 Recovery Protocol(SendMessage retry with FULL context re-paste);二次 idle → coordinator self-review fallback + master report 標 process gap - `comment_to_issue` 一定要實際 post 到 GitHub,不是只在對話中顯示 - **絕對禁用** `subagent_type=Explore` for reviewer agents — Explore 是 read-only,**沒有 Write tool**,無法寫 findings 檔(#47 incident proved this; per #52 v2.59.0+ 強制 general-purpose) @@ -499,7 +500,7 @@ fi ```bash # PR mode git diff --stat origin/$DEFAULT_BRANCH...HEAD # PR head 已 checkout -gh pr diff $PR --repo $GITHUB_REPO > /tmp/diff_$NUMBER.patch +gh pr diff $PR --repo $GITHUB_REPO > "$VERIFY_DIR/diff.patch" # 本地 mode git diff --stat # uncommitted @@ -511,7 +512,7 @@ git diff --stat origin/$DEFAULT_BRANCH...$BRANCH # 取 issue（每個 ref'd issue 都要抓） for I in $REFD_ISSUES; do - gh issue view $I --repo $GITHUB_REPO --json title,body > /tmp/issue_$I.json + gh issue view $I --repo $GITHUB_REPO --json title,body > "$VERIFY_DIR/issue_$I.json" done ``` @@ -538,7 +539,7 @@ Exit code: > > **絕對禁用** `subagent_type=Explore` for reviewer agents。Explore agent 是 read-only（per Agent tool docs：「All tools except Agent, ExitPlanMode, Edit, Write, NotebookEdit」），**沒有 Write tool**，無法寫 findings 檔。`#47` verify 真實發生過：spawn 5 個 Explore agents，5 個全部 idle without output，verify 退化成 1-AI (Codex only)。 > -> **正確選擇** `subagent_type=general-purpose`：含完整 tool set (Read/Grep/Glob/Bash/**Write**/Edit)，可寫 `/tmp/verify__findings_.md`。 +> **正確選擇** `subagent_type=general-purpose`：含完整 tool set (Read/Grep/Glob/Bash/**Write**/Edit)，可寫 `$VERIFY_DIR/findings_.md`。 > > **不用 TeamCreate**（pre-v2.59.0 model）的原因： > - TeamCreate teammates 必須在 `tools` field 顯式列出 Write，現有 prompt template 配置只給 Read/Grep/Glob/Bash —— 同 Explore 一樣 Write-missing failure mode @@ -551,42 +552,53 @@ Exit code: 每個 reviewer 用 single `Agent` tool call（**not** TeamCreate teammate）。所有 5 個 + 1 個 Bash codex **必須在同一個 message** 一起發出（單 message 多 tool calls = parallel）。 -> **Scratch-file naming (#288)**: every `/tmp` path in this skill MUST include a -> per-run token, not just the issue number. `/tmp/verify_${NUMBER}_findings_*.md` -> and `/tmp/codex-verify-${NUMBER}.md` carry no repo identity, so two sessions -> verifying **the same issue number in different repos** share filenames — and a -> leftover file from the earlier run is then read as this run's findings. That +> **Scratch-file naming (#288)**: no scratch path in this skill may be a fixed +> name in the system temp directory. (The superseded ones are not spelled out +> here, for the same reason the prose-drift test bans quoting the classifier +> regex: a copy cannot drift if it does not exist, and a banned literal written +> into the document is the first thing a mechanical check trips over.) They were +> keyed on the issue number alone, which carries no repo identity, so two sessions +> verifying **the same issue number in different repos** share filenames, and a +> leftover file from the earlier run is read as this run's findings. That > failure is silent and its direction is the worst one: the coordinator merges > another repo's verdict into this PR's report. > -> Resolve a run directory ONCE, before spawning anything, and use it everywhere: +> `$VERIFY_DIR` is resolved ONCE in Step 0 (`resolve_scratch_dir`) and every path +> in this skill hangs off it: > > ```bash > VERIFY_DIR=$(mktemp -d "${TMPDIR:-/tmp}/idd-verify-${NUMBER}-XXXXXX") -> # every findings / prompt / codex path below becomes "$VERIFY_DIR/.md" > ``` > > `mktemp -d` gives collision-freedom without needing a repo slug, survives > concurrent runs of the same issue in the same repo, and is cleaned up by the -> OS. Do NOT paper over this by adding a repo slug to the old flat names — -> concurrent runs of the *same* repo would still collide. - -> **Pre-spawn prompt persistence (per /idd-verify --pr 73 round 1 P1.2)**: BEFORE invoking the 5 Agent calls, coordinator MUST save each reviewer's full prompt to `/tmp/verify_${NUMBER}_prompt_.md`. Step 2.5b Recovery Protocol re-paste step reads these files; if they don't exist, retry fails. Save via: +> OS. Do NOT paper over this by adding a repo slug to flat names — concurrent +> runs of the *same* repo would still collide. +> +> **This was documented before it was done.** #288's closing summary claimed +> every scratch path now hung off `mktemp -d`; the note above shipped, and the +> next paragraph went on MUSTing the old flat path, as did twenty-two others. +> A spec that states a rule and then contradicts it a line later is worse than +> one that states nothing: a reader who checks the rule finds it, and a reader +> who copies the command gets the defect. The paths are now actually converted, +> and `scripts/tests/verify-scratch-paths/` fails if a flat one comes back. + +> **Pre-spawn prompt persistence (per /idd-verify --pr 73 round 1 P1.2)**: BEFORE invoking the 5 Agent calls, coordinator MUST save each reviewer's full prompt to `$VERIFY_DIR/prompt_.md`. Step 2.5b Recovery Protocol re-paste step reads these files; if they don't exist, retry fails. Save via: > > ```bash > # Coordinator runs BEFORE Agent invocations -> cat > /tmp/verify_${NUMBER}_prompt_requirements.md <<'EOF' +> cat > $VERIFY_DIR/prompt_requirements.md <<'EOF' > 你是 Requirements Reviewer for Issue #... > (full prompt body here, exactly as passed to Agent below) > EOF > # ... same for logic, security, regression, devils-advocate > ``` > -> Do this once per verify invocation (paths include `${NUMBER}` so different issues don't collide). The 5 prompt files + 5 findings files share the same `verify__*` naming convention. +> Do this once per verify invocation. The 5 prompt files and 5 findings files all live in `$VERIFY_DIR`, so nothing collides — not across issues, not across repos, not across two concurrent runs of the same issue. **Prompt template 強制要素**（每個 reviewer 都必含這 3 條，違反 = process gap）： -1. **明示 file output path**：`Write your findings to /tmp/verify_${NUMBER}_findings_.md when done.` +1. **明示 file output path**：`Write your findings to $VERIFY_DIR/findings_.md when done.` 2. **明示 DO NOT idle**：`Your task is NOT complete until the file is written. Do NOT idle without producing the output file.` 3. **明示 retry context expectation**：`If you receive a later SendMessage with the same prompt re-pasted, treat that as a retry signal; the original context may have been lost across an idle/wake cycle.` @@ -600,14 +612,14 @@ Agent({ Issue body: ${BODY} -Diff path: /tmp/diff_${NUMBER}.patch +Diff path: $VERIFY_DIR/diff.patch Attachment paths (if any): .claude/.idd/attachments/issue-${NUMBER}/... 你的任務：逐一檢查 issue 的每個要求是否在 code 中被實現。 對每個要求標記：FULLY / PARTIALLY / NOT addressed。 用 Read/Grep 工具實際去看相關檔案確認。 -OUTPUT (mandatory): Write your findings to /tmp/verify_${NUMBER}_findings_requirements.md when done. +OUTPUT (mandatory): Write your findings to $VERIFY_DIR/findings_requirements.md when done. Your task is NOT complete until the file is written. Do NOT idle without producing the output file. If you receive a later SendMessage with the same prompt re-pasted, treat that as a retry signal — the original context may have been lost across an idle/wake cycle.` }) @@ -618,7 +630,7 @@ Agent({ model: "${AGENT_MODEL}", // #205: 顯式 dispatch model（Step 2 前解析；預設 opus） prompt: `你是 Logic Reviewer for Issue #${NUMBER}: ${TITLE}. -Diff path: /tmp/diff_${NUMBER}.patch +Diff path: $VERIFY_DIR/diff.patch 你的任務：檢查邏輯正確性。 - Edge cases（null、empty、boundary values） @@ -626,7 +638,7 @@ Diff path: /tmp/diff_${NUMBER}.patch - 控制流程（if/else 覆蓋、switch fall-through） 用 Read 工具查看完整函數上下文。 -OUTPUT (mandatory): Write findings to /tmp/verify_${NUMBER}_findings_logic.md. +OUTPUT (mandatory): Write findings to $VERIFY_DIR/findings_logic.md. Your task is NOT complete until the file is written. Do NOT idle without producing output. If you receive a later SendMessage with the same prompt re-pasted, treat as retry signal.` }) @@ -637,7 +649,7 @@ Agent({ model: "${AGENT_MODEL}", // #205: 顯式 dispatch model（Step 2 前解析；預設 opus） prompt: `你是 Security Reviewer for Issue #${NUMBER}: ${TITLE}. -Diff path: /tmp/diff_${NUMBER}.patch +Diff path: $VERIFY_DIR/diff.patch 你的任務：檢查安全問題。 - SQL injection（字串拼接 vs parameterized） @@ -645,7 +657,7 @@ Diff path: /tmp/diff_${NUMBER}.patch - 權限檢查 - 輸入驗證 -OUTPUT (mandatory): Write findings to /tmp/verify_${NUMBER}_findings_security.md. +OUTPUT (mandatory): Write findings to $VERIFY_DIR/findings_security.md. Your task is NOT complete until the file is written. Do NOT idle without producing output. If you receive a later SendMessage with the same prompt re-pasted, treat as retry signal.` }) @@ -656,7 +668,7 @@ Agent({ model: "${AGENT_MODEL}", // #205: 顯式 dispatch model（Step 2 前解析；預設 opus） prompt: `你是 Regression Reviewer for Issue #${NUMBER}: ${TITLE}. -Diff path: /tmp/diff_${NUMBER}.patch +Diff path: $VERIFY_DIR/diff.patch 你的任務： 1. 有沒有改到 issue 範圍外的東西（scope creep）？ @@ -664,7 +676,7 @@ Diff path: /tmp/diff_${NUMBER}.patch 3. 有沒有引入新的 dependency 但沒處理？ 用 Grep 搜尋被改動的函數在哪裡被呼叫。 -OUTPUT (mandatory): Write findings to /tmp/verify_${NUMBER}_findings_regression.md. +OUTPUT (mandatory): Write findings to $VERIFY_DIR/findings_regression.md. Your task is NOT complete until the file is written. Do NOT idle without producing output. If you receive a later SendMessage with the same prompt re-pasted, treat as retry signal.` }) @@ -675,7 +687,7 @@ Agent({ model: "${AGENT_MODEL}", // #205: 顯式 dispatch model（Step 2 前解析；預設 opus） prompt: `你是 Devil's Advocate for Issue #${NUMBER}: ${TITLE}. -Diff path: /tmp/diff_${NUMBER}.patch +Diff path: $VERIFY_DIR/diff.patch 你是在 4 份 lens findings 檔就緒後才被 spawn 的（coordinator 已確認 — #130 sequenced 模式，無需 polling）。直接讀取 4 份 sibling findings，然後： @@ -685,7 +697,7 @@ Diff path: /tmp/diff_${NUMBER}.patch 這是對抗性驗證 — 你的存在是為了防止群體盲點。 -OUTPUT (mandatory): Write findings to /tmp/verify_${NUMBER}_findings_devils-advocate.md. +OUTPUT (mandatory): Write findings to $VERIFY_DIR/findings_devils-advocate.md. Your task is NOT complete until the file is written. Do NOT idle without producing output. If you receive a later SendMessage with the same prompt re-pasted, treat as retry signal.` }) @@ -693,17 +705,17 @@ If you receive a later SendMessage with the same prompt re-pasted, treat as retr #### 2b. Codex（背景執行，via vendored `codex-call` HTTP wrapper，#147） -透過 pai 的 `codex-call`（`$PAI_CODEX_CALL`；HTTP，非 `codex exec` subprocess → 無 pipe hang）執行 review，model/effort 帶共通前置解析的 codex-pro 治理值（#264）。**注意語意差異**：`codex exec --full-auto` 是 agentic（codex 自己讀 working-tree diff）；`codex-call` 是單次 completion（非 agentic），所以 diff 必須**顯式**用 `--prompt-file` 餵進去 —— 用 Step 1 已寫好的 `/tmp/diff_$NUMBER.patch`，review 框架放 `--instructions`： +透過 pai 的 `codex-call`（`$PAI_CODEX_CALL`；HTTP，非 `codex exec` subprocess → 無 pipe hang）執行 review，model/effort 帶共通前置解析的 codex-pro 治理值（#264）。**注意語意差異**：`codex exec --full-auto` 是 agentic（codex 自己讀 working-tree diff）；`codex-call` 是單次 completion（非 agentic），所以 diff 必須**顯式**用 `--prompt-file` 餵進去 —— 用 Step 1 已寫好的 `"$VERIFY_DIR/diff.patch"`，review 框架放 `--instructions`： ```bash Bash({ - command: `"$PAI_CODEX_CALL" --output /tmp/codex-verify-$NUMBER.md --model "$CODEX_MODEL" --effort "$CODEX_EFFORT" --service-tier fast --max-time "$CODEX_MAX_TIME" --prompt-file /tmp/diff_$NUMBER.patch --instructions "You are verifying code changes for Issue #$NUMBER: $TITLE. Go through EACH requirement: FULLY / PARTIALLY / NOT addressed. Flag scope creep and regressions. Reply in Traditional Chinese."`, + command: `"$PAI_CODEX_CALL" --output $VERIFY_DIR/codex.md --model "$CODEX_MODEL" --effort "$CODEX_EFFORT" --service-tier fast --max-time "$CODEX_MAX_TIME" --prompt-file "$VERIFY_DIR/diff.patch" --instructions "You are verifying code changes for Issue #$NUMBER: $TITLE. Go through EACH requirement: FULLY / PARTIALLY / NOT addressed. Flag scope creep and regressions. Reply in Traditional Chinese."`, description: "Codex review for #$NUMBER (via codex-call)", run_in_background: true }) ``` -完成後用 Read 讀取 `/tmp/codex-verify-$NUMBER.md`。codex-call 失敗（swift 缺 / HTTP / auth / timeout）→ 視為 cross-model lens 本次 skip，標記在 master report，不靜默當成 PASS。 +完成後用 Read 讀取 `$VERIFY_DIR/codex.md`。codex-call 失敗（swift 缺 / HTTP / auth / timeout）→ 視為 cross-model lens 本次 skip，標記在 master report，不靜默當成 PASS。 ### Step 2.5: Recovery Protocol（NEW v2.59.0+, #52） @@ -715,11 +727,11 @@ Bash({ ```bash EXPECTED_FILES=( - "/tmp/verify_${NUMBER}_findings_requirements.md" - "/tmp/verify_${NUMBER}_findings_logic.md" - "/tmp/verify_${NUMBER}_findings_security.md" - "/tmp/verify_${NUMBER}_findings_regression.md" - "/tmp/verify_${NUMBER}_findings_devils-advocate.md" + "$VERIFY_DIR/findings_requirements.md" + "$VERIFY_DIR/findings_logic.md" + "$VERIFY_DIR/findings_security.md" + "$VERIFY_DIR/findings_regression.md" + "$VERIFY_DIR/findings_devils-advocate.md" ) MISSING_ROLES=() @@ -775,7 +787,7 @@ for role in "${MISSING_ROLES[@]}"; do # Build retry prompt with full context re-paste RETRY_PROMPT="[RETRY] Original prompt re-pasted because the previous instance idled without producing output. Treat this as the canonical task instruction; do not assume any prior context. -$(cat /tmp/verify_${NUMBER}_prompt_${role}.md)" # Coordinator saved prompts before spawn +$(cat $VERIFY_DIR/prompt_${role}.md)" # Coordinator saved prompts before spawn # If Agent instance is addressable via SendMessage (named team member or running agent): SendMessage(to="verify-${NUMBER}-${role}", body="$RETRY_PROMPT") @@ -790,7 +802,7 @@ $(cat /tmp/verify_${NUMBER}_prompt_${role}.md)" # Coordinator saved prompts be # Poll for file (90s max) for i in $(seq 1 18); do - [ -s "/tmp/verify_${NUMBER}_findings_${role}.md" ] && break + [ -s "$VERIFY_DIR/findings_${role}.md" ] && break sleep 5 done done @@ -804,13 +816,13 @@ done ```bash for role in "${MISSING_ROLES[@]}"; do - if [ ! -s "/tmp/verify_${NUMBER}_findings_${role}.md" ]; then + if [ ! -s "$VERIFY_DIR/findings_${role}.md" ]; then # Coordinator self-review echo "## ${role} review (coordinator self-review — process gap)" \ - > "/tmp/verify_${NUMBER}_findings_${role}.md" - echo "" >> "/tmp/verify_${NUMBER}_findings_${role}.md" + > "$VERIFY_DIR/findings_${role}.md" + echo "" >> "$VERIFY_DIR/findings_${role}.md" echo "(${role} Agent failed to produce output after retry. Coordinator self-reviewed:)" \ - >> "/tmp/verify_${NUMBER}_findings_${role}.md" + >> "$VERIFY_DIR/findings_${role}.md" # Coordinator reads diff + issue + does role-specific review inline # (Quality lower than independent reviewer; flagged as process gap.) @@ -859,8 +871,8 @@ fi 等 5 reviewer Agents（Step 2.5 Recovery Protocol 已 satisfy: 所有 findings 檔 present + non-empty）和 Codex 都完成後： -1. 收集 5 個 reviewer Agents 的 findings（從 `/tmp/verify_${NUMBER}_findings_*.md`） -2. 收集 Codex 的 findings（從 `/tmp/codex-verify-${NUMBER}.md`） +1. 收集 5 個 reviewer Agents 的 findings（從 `$VERIFY_DIR/findings_*.md`） +2. 收集 Codex 的 findings（從 `$VERIFY_DIR/codex.md`） 3. **去重**：相同檔案 + 相似描述 → 合併，標註來源 `[agents:logic+codex]` 4. **severity 以最高為準**：如果 logic 說 P2 但 codex 說 P1 → P1 5. Devil's Advocate 的反駁如果成立 → 升級 severity @@ -879,14 +891,14 @@ fi ```bash # 1. Post master to PR, capture URL -MASTER_URL=$(gh pr comment $PR --repo $GITHUB_REPO --body-file /tmp/master.md 2>&1 | tail -1) +MASTER_URL=$(gh pr comment $PR --repo $GITHUB_REPO --body-file "$VERIFY_DIR/master.md" 2>&1 | tail -1) # 2. Compose pointer body using captured PR comment URL -sed "s|__MASTER_URL__|$MASTER_URL|g" /tmp/pointer_template.md > /tmp/pointer.md +sed "s|__MASTER_URL__|$MASTER_URL|g" "$VERIFY_DIR/pointer_template.md" > "$VERIFY_DIR/pointer.md" # 3. Post pointer to each ref'd issue in parallel for I in $REFD_ISSUES; do - bash "$CLAUDE_PLUGIN_ROOT/scripts/gh-egress.sh" comment $I --repo $GITHUB_REPO --body-file /tmp/pointer.md --scrub-attested "$SCRUB_LEVEL" & + bash "$CLAUDE_PLUGIN_ROOT/scripts/gh-egress.sh" comment $I --repo $GITHUB_REPO --body-file "$VERIFY_DIR/pointer.md" --scrub-attested "$SCRUB_LEVEL" & done wait ``` @@ -920,15 +932,18 @@ Cluster（≥2 issue 共用一份 verify report）： **Helper pattern**: ```bash +# All three files live in $VERIFY_DIR (Step 0). These are EGRESS bodies — the +# text that gets posted to someone's issue — so a stale or half-written file at +# a shared fixed path does not fail loudly, it publishes the wrong comment. # 1. Post master, capture URL -MASTER_URL=$(bash "$CLAUDE_PLUGIN_ROOT/scripts/gh-egress.sh" comment $HUB_ISSUE --repo $REPO --body-file /tmp/master.md --scrub-attested "$SCRUB_LEVEL" 2>&1 | tail -1) +MASTER_URL=$(bash "$CLAUDE_PLUGIN_ROOT/scripts/gh-egress.sh" comment $HUB_ISSUE --repo $REPO --body-file "$VERIFY_DIR/master.md" --scrub-attested "$SCRUB_LEVEL" 2>&1 | tail -1) # 2. Compose pointer body using captured URL -sed "s|__MASTER_URL__|$MASTER_URL|g" /tmp/pointer_template.md > /tmp/pointer.md +sed "s|__MASTER_URL__|$MASTER_URL|g" "$VERIFY_DIR/pointer_template.md" > "$VERIFY_DIR/pointer.md" # 3. Post pointers in parallel for I in $POINTER_ISSUES; do - bash "$CLAUDE_PLUGIN_ROOT/scripts/gh-egress.sh" comment $I --repo $REPO --body-file /tmp/pointer.md --scrub-attested "$SCRUB_LEVEL" & + bash "$CLAUDE_PLUGIN_ROOT/scripts/gh-egress.sh" comment $I --repo $REPO --body-file "$VERIFY_DIR/pointer.md" --scrub-attested "$SCRUB_LEVEL" & done wait ``` @@ -1133,11 +1148,12 @@ Full integration contract: [`references/agent-routing.md`](../../references/agen ```bash # codex-call 是單次 completion（非 agentic）→ 顯式把 diff 餵進去 -git diff > /tmp/codex-quick-diff.patch +QUICK_DIR=$(mktemp -d "${TMPDIR:-/tmp}/idd-quick-XXXXXX") +git diff > "$QUICK_DIR/diff.patch" "$PAI_CODEX_CALL" \ - --output /tmp/codex-quick-review.md \ + --output "$QUICK_DIR/review.md" \ --model "$CODEX_MODEL" --effort "$CODEX_EFFORT" --service-tier fast --max-time "$CODEX_MAX_TIME" \ - --prompt-file /tmp/codex-quick-diff.patch \ + --prompt-file "$QUICK_DIR/diff.patch" \ --instructions "Review this git diff. Flag bugs, logic errors, security issues. Reply in Traditional Chinese." ``` From 488ba395ad5b5d3b740810dd2ed44a3e52b2109f Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:48:01 +0800 Subject: [PATCH 07/11] =?UTF-8?q?fix:=20idd-clarify=20=E7=9A=84=20worked?= =?UTF-8?q?=20example=20=E7=AC=AC=E4=B8=89=E6=AC=84=E9=82=84=E6=98=AF?= =?UTF-8?q?=E8=88=8A=E7=9A=84=E5=88=86=E6=9E=90=E5=BC=8F=E5=AF=AB=E6=B3=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #294 依使用者原話把 `Question for you` 那一欄從描述式分析改成問句 —— > 「iddclarify我覺得你可以直接放對使用者的問題」 理由在讀者是誰：那一欄給的是**要做裁決的人**。寫成分析，他讀完還得先把分析 還原成一個可回答的問題。 出貨的是**規則**、一張 ❌/✅ 對照表示範規則 —— 然後四十行後的 worked example 第三欄還是分析（`分群變數 / distinguishing variable (per K-means context…)`）。 照規則做的人得到新行為，照範例抄的人得到舊行為，而**範例才是會被抄的那半**。 跟 #288 的暫存路徑同一類：先寫規則、再在自己的範例裡違反它。 改掉兩列，並新增 `scripts/tests/clarify-question-column/`（第 49 個 suite）： 掃所有 `surfaced` 範例列的第三欄，沒有問號就紅（`...` 佔位、`passed`、 `deferred` 三種豁免——前者不是措辭範例，後兩者沒有東西可問）。帶 positive control（唯一檔名 + trap）。在修改前的樹上跑會紅，已驗。 全 suite 49/49。 --- .../tests/clarify-question-column/test.sh | 55 +++++++++++++++++++ .../skills/idd-clarify/SKILL.md | 4 +- 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100755 plugins/issue-driven-dev/scripts/tests/clarify-question-column/test.sh diff --git a/plugins/issue-driven-dev/scripts/tests/clarify-question-column/test.sh b/plugins/issue-driven-dev/scripts/tests/clarify-question-column/test.sh new file mode 100755 index 0000000..e1de5bb --- /dev/null +++ b/plugins/issue-driven-dev/scripts/tests/clarify-question-column/test.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# Test: the `Question for you` column contains QUESTIONS (#294). +# +# WHY THIS EXISTS +# +# #294 changed that column from descriptive analysis to a question, on the +# user's own instruction ("iddclarify我覺得你可以直接放對使用者的問題"). The +# reason is who reads it: the person making the call. Analysis makes them +# reconstruct the question before they can answer it. +# +# What shipped was the RULE, an ❌/✅ contrast table demonstrating the rule — +# and, forty lines further down, a worked example whose third column was still +# analysis (`分群變數 / distinguishing variable (per K-means context…)`). A +# reader following the example gets the old behaviour; a reader following the +# rule gets the new one. The example is the part people copy. +# +# Same class as #288's scratch paths: a document that states a rule and then +# violates it in its own example is worse than one that states nothing. +set -u +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PLUGIN="$(cd "$HERE/../../.." && pwd)" +. "$(cd "$HERE/../../lib" && pwd)/assert-helpers.sh" + +# Every example row with status `surfaced` must ask something. Placeholder rows +# (`...`), `passed` rows and `deferred` rows are exempt: the first are not +# examples of wording, and the last two have nothing to ask. +bad_rows() { + grep -rhE --include='*.md' '^\| (terminology|ambiguity|missing-context) \|' "$PLUGIN/skills" 2>/dev/null \ + | grep -E '\| surfaced \|' \ + | awk -F'|' '{q=$4; gsub(/^[ \t]+|[ \t]+$/,"",q); if (q != "..." && q !~ /[?？]/) print}' +} + +HITS=$(bad_rows || true) +require "every surfaced example row asks a question, not states an analysis" \ + bash -c '[ -z "$0" ] || { printf "%s\n" "$0"; exit 1; }' "$HITS" + +# Positive control: plant a row in the old analysis style and prove the scan +# sees it. Unique name + trap — the canary is written into the repo. +CANARY="$PLUGIN/skills/idd-clarify/.question-canary.$$-${RANDOM}.md" +trap 'rm -f "$CANARY"' EXIT HUP INT TERM +printf '| terminology | "x" | 語義邊界未定義 | surfaced |\n' > "$CANARY" +SEEN=$(bad_rows | grep -c '語義邊界未定義' || true) +rm -f "$CANARY" +require "positive control: the scan detects an analysis-style row" \ + bash -c '[ "$0" -ge 1 ]' "$SEEN" + +# And the rule itself must still be stated — the examples above conform to it, +# but conformance without the rule written down is one edit away from drifting +# back, and nothing here would notice. +CLARIFY=$(cat "$PLUGIN/skills/idd-clarify/SKILL.md") +assert_grep "the question-not-analysis rule is stated" "第三欄寫「問句」，不是分析" "$CLARIFY" +assert_grep "...and requires a one-sentence answer" "可以用一句話回答" "$CLARIFY" + +print_summary "clarify-question-column" +exit $? diff --git a/plugins/issue-driven-dev/skills/idd-clarify/SKILL.md b/plugins/issue-driven-dev/skills/idd-clarify/SKILL.md index 522e148..196de37 100644 --- a/plugins/issue-driven-dev/skills/idd-clarify/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-clarify/SKILL.md @@ -415,8 +415,8 @@ Annotation appended: | Type | Source | Question for you | Status | |---|---|---|---| -| terminology | "可否 prompt 跟他說各群要有至少一個最高得分的特徵值" | 分群變數 / distinguishing variable (per K-means context, library row 1) | surfaced | -| missing-context | "請根據上面網址的Ｋ欄的情感、人、場..." | customer × attribute score 來源未指定(GSheet 只有 metadata) | surfaced | +| terminology | "可否 prompt 跟他說各群要有至少一個最高得分的特徵值" | 這裡的「特徵值」指的是分群變數（distinguishing variable）嗎？ | surfaced | +| missing-context | "請根據上面網址的Ｋ欄的情感、人、場..." | customer × attribute 的分數要從哪裡讀？那份 GSheet 看起來只有 metadata。 | surfaced | ``` ### Example 2: dismiss a false positive From 3fc1bde2723b275537004ebc9c76b91a1f0df087 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:49:22 +0800 Subject: [PATCH 08/11] =?UTF-8?q?chore:=202.110.0=20=E2=80=94=20CHANGELOG?= =?UTF-8?q?=20+=20plugin.json/marketplace.json=20=E5=90=8C=E6=AD=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude-plugin/marketplace.json | 2 +- .../.claude-plugin/plugin.json | 2 +- plugins/issue-driven-dev/CHANGELOG.md | 72 +++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 184a6df..13b1d8d 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -15,7 +15,7 @@ "plugins": [ { "name": "issue-driven-dev", - "version": "2.109.0", + "version": "2.110.0", "description": "v2.102.2: Deep Research light integration (#277, ruling b). idd-diagnose gains a non-binding pointer (the #111 superpowers hand-off shape: pure suggestion, no presence check, no dependency) fired when the diagnosis's quality depends on facts OUTSIDE the repo — with trigger examples AND counter-examples (the overly-broad-signal risk). Output flows back via '/idd-comment --type note' as summary + link, never full text (#116) — that is what keeps external research inside the audit trail. Both real-user misconceptions get canonical answers where they lived: research attaches AT diagnose (not after plan), and research vs implement are different phases' work, not substitutes. usecase-routing scenario 32 + a three-row internal-corpus vs external-world boundary table (idd-find / idd-ask / Deep Research). Deep integration stays a recorded residue until a plugin-dependable primitive exists. v2.102.1: reopen / resume path (#278) — the legal return trip from closed. idd-close gains a 'Reopen / resume path' section (close's dual operation): reopen-vs-new-issue criteria (same Expected -> reopen for trail continuity; morphed need -> new issue Refs old; broken upstream artifact -> #200's re-baseline, out of this path), resume point decided by the closing summary's WHY (premise changed -> re-diagnose; pure deferral -> implement), and the old summary stays untouched (append-only; reopen = note comment + idd-update phase rollback + optional prepend-note). Cross-referenced against auto-close-trap recovery. usecase-routing scenario 31. From a real user exchange; verify on substitute basis (disclosed). v2.102.0: three-front release. Skill-description contract + Path Map (#276, two-phase): idd-plan's frontmatter description — the ONLY surface read at skill-selection time — now names its diagnosis precondition; 5 skills gain the house pattern (drift-guard skill-description-contract, RED 8 first); docs/workflows.md gains a mermaid Path Flowchart mirroring the decision tree with all 36 catalog paths, rendered deterministically to the wiki Path-Map page by scripts/generate-path-map.py (drift-guard path-map-sync: freshness / coverage / discovery). Egress data-safety cluster (#275 + #273): empty-body guard — a provided-but-empty body now refuses (exit 15, band discipline; edit floors at 10 stripped chars because overwrite semantics turn empty dispatch into data loss — live incident 2026-07-22; explicit-intent escape --allow-empty-body); and the comment-PATCH surgery channel enters the nets via the new edit-comment verb (the #226 rollout's tracked-separately whitelist debt retired — it had bypassed EVERY net), with idd-edit's batch loop consuming the refusal band into a second outcome bucket (final exit stays 4). Dogfood: the #163 contract layer caught this release's own SCRUB_LEVEL provenance gap on first sweep. 42 suites, 0 fail. v2.99.1: staleness sweep + guard-net expansion (#267). README carried three stale gpt-5.5 pins and a stale vendored-codex-call claim — all outside the drift-guard scan net; fixed and the net widened: model-generation-sync now refutes pins in README + both catalog docs (31 assertions), and a new docs-catalog-sync suite requires every skills/* directory to appear in the catalog docs (the #122 no-forcing-function root cause is now test-detectable; it caught idd-ask and idd-config on its first RED). docs/workflows.md + skill-dimensions.md backfilled to v2.99 reality (P-find-lookup / P-ask-history / P-report-rollup / P-config-maintain / P-verify-file-profile paths, matrix rows, D12 4th member). 38 suites 0 fail. v2.99.0: /idd-ask — grounded QA over the issue corpus (#72), the surfacing family's 4th member mirroring /spectra-ask. Natural-language question -> decide-to-search gate (greetings/meta skip; bug-shaped questions never trigger diagnose) -> retrieval delegating idd-find's search backend (family rule: never rebuild a read-only query) -> full-text read of top-N hits (default 5, capped 10) -> grounded synthesis: first line blockquotes the question, every claim carries an issue/comment citation, source priority closed-with-PR > open > orphaned comment with conflicts surfaced, ending with Referenced Issues; corpus silence reported honestly, never filled from training memory. Read-only allowed-tools locked. First live run of the #140 fourth-member procedure (Q3 weak-hit judgment recorded in the family canonical). New capability spec idd-ask (+2 requirements); new drift-guard suite; 37 suites 0 fail. v2.98.0: codex channel goes full-dependency (#264, user ruling 'like superpowers'). The vendored bin/codex-call is DELETED — it trailed pai 2.18.0 by four security/correctness fixes (token-exp NSNumber parse, OAuth-file umask 0o077, form-encoding escape, post-flock re-read). Executable now resolves from the parallel-ai-agents plugin cache (MIN_PAI 2.19.0 — the codexModel/codexEffort contract floor, pai issue 22); model/effort/max-time governance resolves from codex-pro's EXTERNAL-CONSUMER CONTRACT (MIN_CODEX_PRO 0.7.0: machine-readable references/defaults.json base + global/project profile.yaml overlay, codex-pro issue 7) and is passed explicitly on all three call paths (canonical Workflow args + manual fan-out + legacy direct). IDD's tree contains ZERO model pins — generation bumps touch codex-pro's defaults.json only. Dependency wiring mirrors the superpowers shape: install-time dependencies entry (codex-pro@codex-pro), allowCrossMarketplaceDependenciesOn, check-plugin-presence pre-flight, fail-fast with a one-step install instruction, no soft fallback. model-generation-sync drift-guard reshaped to the v2 contract (a re-vendored codex-call fails the suite). 36 suites 0 fail. v2.97.0: 9-issue drain via 5 cluster PRs (#259-#263). Composable verification profiles (#258): idd-verify --profile code|prose|academic (+ config-registered custom via verify_profiles) switches the (lens set, DA focus, input source, freshness) four-tuple; new --file/--dir input sources make the git worktree optional; file-mode SHA-256 freshness gate mirrors the #228 diff gate (never silently exempted); code default byte-identical. New /idd-find skill (#139): surfacing-only semantic lookup over the open+closed corpus with GitHub relevance + phase/PR overlay; read-only, filter flags redirect to idd-list, embedding honestly deferred. Dashboard comment contract (#133) + idd-report --rollup (#134): one human-facing narrative snapshot per issue (marker-located, updates bound to phase transitions only, anti-#116) and a pull-only four-group attention view (need-attention / in-progress / stalled>14d / recently-closed). sdd_bias config switch (#252): hard-gate hits escalate to Spectra when high; default routing byte-identical. Layer V unattended deferred-record (#120): registry literal + structured catch-up record aggregated by idd-all Phase 6. Surfacing-primitives family doc, D12 axis (#140). Model-generation sync (#251): codex-call default gpt-5.6-sol is the tree's single generation pin (live-probed); prose generation-neutral; idd-route candidate renamed codex-xhigh. Docs path catalog completed (#122). 5 new drift-guard suites; 36 suites 0 fail. v2.96.0: gh-egress hardening cluster + idd-edit batch semantics. Exit-code band >=10 (#227: 10=privacy/11=mention/12=unscannable/13=attestation/14=usage; wrapper never exits <10 on its own — rc<10 is always gh's, so unattended callers can split gate-refusal from gh-failure on $? alone). Unified python3 content-net scan (#225: kills the jq/no-jq divergence; taxonomy = projects keys + path-shaped values under sensitive key names; fail-closed wide net when python3 absent). Phase 2 rollout (#226: all 6 skills' comment/edit egress now dispatch through gh-egress with attestation — the #117 mention net is mechanically enforced on the comment channel). idd-edit batch x R5 (#158: per-comment refuse + continue, batch outcome report, exit 4 iff any refused). v2.95.0: Discussions intake bridge (#221) — opt-in `idd-list --discussions` (GraphQL surface: Q&A/Ideas + unanswered + deduped vs issue refs; graceful no-op) + `idd-issue --from-discussion` (Provenance seed + draft-and-confirm reply, unattended never posts); cardinal rule: never auto-file. Plus idd-verify diff-freshness gate (#228: FROZEN_SHA vs HEAD before aggregate — refuse stale-snapshot verdicts) and the IDD_CALLER registry (#161: dynamic tree-sweep drift-guard). v2.94.0: selective git auto-tag (#85) — idd-issue tags idd-{N}-baseline at main HEAD (rollback anchor); idd-verify tags idd-{N}-verified on Aggregate PASS (review snapshot). Only these two milestones (no diagnose/plan/implement tags) so the tag namespace stays clean. Config `auto_tag` (default-ON, opt-out via enabled:false); idempotent (existing tag skipped) + graceful-skip on push failure (never aborts the workflow). v2.93.1: collaborator identity registry in idd-config (#86) — optional `collaborators[]` config field mapping a person's alias / email / display-name → GitHub @login WITHOUT guessing (github_login required; email is PII, private/gitignored only). tagging-collaborators.md Step 2.5 consults the registry first as an accelerator (a hit is still existence-verified via `gh api users/`; a miss falls through to the API fuzzy-match); idd-config validate checks login charset + globally-unique aliases + PII reminder. v2.93.0: reshape Plan / pre-implementation tier (Cluster C, #129/#57/#111, via reshape-plan-preimpl-tier Spectra change) — first-class `meeting` issue type (meeting-first routing + Phase A/B/C deliberation + self-contained close gate), complexity hard gate (>=5-file interdependent-concept OR shared-abstraction MUST-trigger Plan, escalate-only), and superpowers pre-implementation hand-off (README stage-mapping table + non-binding brainstorming pointer, no self-built staging skill). v2.92.1: hotfix — parallel-ai-agents install-time dependency pointed at the wrong marketplace (psychquant-claude-plugins), making v2.92.0 fail to load and silently dropping all /idd-* skills; corrected to the parallel-ai-agents marketplace. v2.92.0: /idd-all batch-drain release — 23 issues verified+closed via 16 PRs (#223, #229-#243), the plugin's largest self-dogfood. Added: unattended-contract (state-file signal + TTL, TTY heuristic removed, idd-all/chain dependency early gates #123/#222/#211); gh-egress unconditional @-mention net with --mention-attested escape-or-attest contract (#117) atop 6-item mechanical-net precision hardening (#203); idd-close Step 6.3 doc-sync sweep (#220); test aggregator + GitHub Actions CI, 21 suites (#217); idd-list blocked-state grouping + all-blocked banner (#84); config Mechanism 3.5 submodule routing (#162); check-plugin-presence enabled-state detection exit 3 (#212); monorepo host plugin disambiguation (#68); assert-helpers eval-content ban + safe output-grep pair (#188); diagnosis-detection contract fixtures (#61). Changed: parallel-ai-agents promoted to install-time dependency, vendored ensemble fork DELETED, idd-verify two-tier chain (#219); DA sequenced-spawn eliminates the #119 socket-crash polling window (#130); spectra-archive-post-ic --force-linked-issue vs --linked-issue intent separation (#172); worktree conventions unified on the managed helper (#169); bridge state migrated to .claude/.idd/state/bridge.json (#199); .gitattributes LF policy (#216); merge-completeness fixtures default-branch self-sufficiency (#224). Audits: dependency bindings vs deep-integration rule (#210), rules layering 12/12 (#215). Follow-ups filed: #225-#228.", "author": { "name": "Che Cheng" diff --git a/plugins/issue-driven-dev/.claude-plugin/plugin.json b/plugins/issue-driven-dev/.claude-plugin/plugin.json index af911fc..62d04bd 100644 --- a/plugins/issue-driven-dev/.claude-plugin/plugin.json +++ b/plugins/issue-driven-dev/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "issue-driven-dev", "description": "v2.102.2: Deep Research light integration (#277, ruling b). idd-diagnose gains a non-binding pointer (the #111 superpowers hand-off shape: pure suggestion, no presence check, no dependency) fired when the diagnosis's quality depends on facts OUTSIDE the repo — with trigger examples AND counter-examples (the overly-broad-signal risk). Output flows back via '/idd-comment --type note' as summary + link, never full text (#116) — that is what keeps external research inside the audit trail. Both real-user misconceptions get canonical answers where they lived: research attaches AT diagnose (not after plan), and research vs implement are different phases' work, not substitutes. usecase-routing scenario 32 + a three-row internal-corpus vs external-world boundary table (idd-find / idd-ask / Deep Research). Deep integration stays a recorded residue until a plugin-dependable primitive exists.", - "version": "2.109.0", + "version": "2.110.0", "author": { "name": "Che Cheng" }, diff --git a/plugins/issue-driven-dev/CHANGELOG.md b/plugins/issue-driven-dev/CHANGELOG.md index c75879f..1ceff35 100644 --- a/plugins/issue-driven-dev/CHANGELOG.md +++ b/plugins/issue-driven-dev/CHANGELOG.md @@ -5,6 +5,78 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.110.0] - 2026-08-15 + +### Added — the closing-summary gate is now executed, not read + +- **`check-closed-without-summary.sh --issue N`** — a single-issue mode that prints one JSON object and **exits with a + verdict**: `0` = class `missing` on a comment set known to be complete, `1` = any other class, `2` = could not + determine (not CLOSED / truncated / fetch or parse failure). Audit mode's always-exit-0 contract is untouched and + pinned by an assertion; that contract is for reporting to a human, and a gate that always exits 0 is not a gate. + The mode also fetches comments through REST `--paginate` rather than the nested `--json comments` connection, which + is capped at the **oldest** 100 — the audit path repairs that after the fact, the gate never takes the road. + +- **`/idd-close --retroactive` now runs it.** The skill previously carried an honest disclosure that its own gate was + prose — that no runtime would stop an execution which ignored the table, and that the mechanical judgement existed + only in the helper, which the skill **did not call**. So seven verify rounds of classifier work bound the + irreversible path only when an agent happened to read the right table. It now invokes the helper before drafting and + again before posting, and refuses on anything that is not `rc == 0` — including every *uncertain* case, and including + "the helper is missing", since a gate you cannot find is not a gate. + +### Fixed — the rest of the post-merge audit (Codex cross-model pass) + +- **`migrate-idd-config.sh` only handles ordinary files now.** Four shapes it accepted silently: `.idd` as a symlink + (`mkdir -p` accepts a symlink-to-dir and reports success, so the move followed it out of the tree); a destination + that is a **directory** (`mv` moves the file *inside* it and prints `✓ migrated`); a dangling destination symlink + (invisible to `-e`); and a destination symlink to a regular file elsewhere (`-f` follows it, so the audit printed + "both present, current wins" — false whenever "current" is not in this repo). Plus a breadcrumb path that is a + dangling symlink, where `>` *creates* the target. `-f`/`-d`/`-e` all follow symlinks, so `-L` must be tested first. + The move itself is now `ln` + `rm` rather than `mv`: `mv` has no atomic no-clobber, `ln` fails atomically when the + destination exists, and an interruption leaves two links to one inode rather than a file at neither path. `find` is + now shape-limited to `*/.claude/` — matching on the name alone picked up doc samples and fixtures and invented + a `.idd` directory in unrelated trees. + +- **`idd-repo-map.sh` rendered its row buffer with `printf '%b'`**, which expands escapes in the *data*. A backslash in + a path or a `github_repo` value was executed: `\c` stops all output, so one crafted value silently deleted every + later row *and* the totals line — and a map missing half its rows is indistinguishable from a machine with half as + many repos. **This corrects a claim in the previous entry's audit**: `--json` was not exempt, because the same `%b` + feeds jq. Real tab/newline delimiters + `%s` throughout. `find`'s exit status is now checked, too: an unreadable + directory answering "no config here" is exactly the wrong upward resolution this map exists to prevent. + +- **The content test now uses the same notion of "visible" as the lead test.** `lead_line` skips whole-line HTML + markers; `lead_has_content` did not, so `## Closing Summary` followed only by `` classified as + `compliant` — printed in no section at all, and refused by `--retroactive` too. That is the silencing channel the + content test was added to close, reopened one level down. + +- **Permissive matching no longer backs a positive claim.** `idd-find` labelled a quotation `📜 closing summary + （可考古的結案紀錄）`; it now says `📜 summary marker`, which is what a deliberately-loose match can promise. + `idd-update` inferred phase from a permissive match, so a comment merely *quoting* `> ## Closing Summary` pushed an + **open** issue to `closed`; phase is a positive claim, so it now requires the strict lead predicate. + +- **Attachment URLs kept their wrappers.** The extractor excluded `)` and whitespace only — i.e. markdown links and + nothing else. An autolink kept its `>`, an HTML attribute its `">`, a sentence-final URL its full stop. All three + 404, and an attachment that cannot be downloaded is a source that gets ignored. + +- **`#288`'s scratch paths are actually converted.** Its closing summary said every path hung off `mktemp -d`; what + shipped was a *note* saying so, followed by twenty-four paragraphs still naming fixed paths — including the OUTPUT + instructions handed to reviewer agents, and two copies of an egress loop writing the body that gets posted to + someone's issue. `scripts/tests/verify-scratch-paths/` now fails if a fixed one returns. Scope is stated in the test: + it covers `idd-verify` only, not `idd-edit`. + +- **`idd-clarify`'s worked example** still used the analysis style `#294` replaced with questions, forty lines below the + rule and its ❌/✅ demonstration. The example is the half people copy. `scripts/tests/clarify-question-column/` pins it. + +### Honest residue + +- The `.idd` re-check after `mkdir -p` narrows a TOCTOU window and **has no test weight** — a single-process test + cannot reach it. Recorded rather than dressed up. +- The jq-error path strips ASCII controls only; its comment used to claim bidi as well. Measured before changing + anything: three payloads (U+202E, U+2028, ESC) planted where a bare string reaches `.state` produce + `Cannot index string with string ("state")` on jq 1.7 — the value is **not** echoed, so nothing attacker-controlled + arrives. Comment corrected, code left alone: a Unicode-aware scrub would mean a second copy of the character class, + and divergent copies of a safety definition are this file's longest-running failure. +- Suites: 47 → 49. Classifier assertions: 89 → 120. + ## [2.109.0] - 2026-08-15 ### Fixed — post-merge audit of the 2026-08-13/14 session From 0d10a93758ec8ffe2b0e0cf6ff413f742b0c92c6 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:53:08 +0800 Subject: [PATCH 09/11] =?UTF-8?q?fix:=20idd-plan=20=E5=B0=8D=20idd-all=20?= =?UTF-8?q?=E7=9A=84=20Plan=20routing=20=E6=8F=8F=E8=BF=B0=E6=94=B9?= =?UTF-8?q?=E6=88=90=E5=88=86=E8=BF=B0=EF=BC=88#317=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #292 之後 **attended** 的 `/idd-all` 走 `/idd-plan`（`EnterPlanMode` 閘門住在 那裡、不在 `/idd-implement`）。`idd-all` 更新了，`idd-plan` 的「與 idd-all 的 整合」段沒有，而且它把舊規則寫成**全稱句**： > **idd-all 不該走 Plan path**…應該 fallback 走 Simple path 那句話的 unattended 半段是對的。錯在把一個分支的結論寫成無條件的 —— 以 idd-plan 為準的讀者，會得到與實際執行相反的結論。 改成兩欄分述（attended / unattended），並明講 normative source 是 `idd-all` 的 dispatch table。這正是 repo 自己那條規則說的：**能列舉的就列舉，不要先寫一句 總括判準再補例外** —— 總括與例外是兩份不會一起改的規格。 新增 `scripts/tests/plan-routing-consistency/`（第 50 個 suite）：先對 `idd-all` 斷言 routing 本身仍成立（否則下面那些斷言只是在釘一份虛構），再對 `idd-plan` 斷言分述存在、全稱句消失。在修改前的樹上跑紅 4 條，已驗。 全 suite 50/50。 --- .../tests/plan-routing-consistency/test.sh | 40 +++++++++++++++++++ .../issue-driven-dev/skills/idd-plan/SKILL.md | 18 ++++----- 2 files changed, 49 insertions(+), 9 deletions(-) create mode 100755 plugins/issue-driven-dev/scripts/tests/plan-routing-consistency/test.sh diff --git a/plugins/issue-driven-dev/scripts/tests/plan-routing-consistency/test.sh b/plugins/issue-driven-dev/scripts/tests/plan-routing-consistency/test.sh new file mode 100755 index 0000000..60ed025 --- /dev/null +++ b/plugins/issue-driven-dev/scripts/tests/plan-routing-consistency/test.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# Test: idd-plan's account of idd-all's Plan routing matches idd-all (#317). +# +# WHY THIS EXISTS +# +# #292 moved Plan-tier routing so that ATTENDED /idd-all calls /idd-plan (the +# EnterPlanMode gate lives there, not in /idd-implement). idd-all was updated; +# idd-plan's "與 idd-all 的整合" section was not, and it stated the old rule as +# a UNIVERSAL: "idd-all 不該走 Plan path". Its unattended half was right — the +# error was writing one branch's conclusion without its condition. A reader +# taking idd-plan as the source concluded the opposite of what runs. +# +# Same failure the repo's own doctrine warns about: a blanket judgement plus +# examples is two specifications that will not be edited together. Enumerate. +set -u +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PLUGIN="$(cd "$HERE/../../.." && pwd)" +. "$(cd "$HERE/../../lib" && pwd)/assert-helpers.sh" + +PLAN=$(cat "$PLUGIN/skills/idd-plan/SKILL.md") +ALL=$(cat "$PLUGIN/skills/idd-all/SKILL.md") + +# The normative side first. If idd-all ever stops routing attended Plan to +# /idd-plan, the reader-side assertions below are pinning fiction — so they are +# asserted against the source, not assumed. +assert_grep "idd-all routes attended Plan tier to /idd-plan" \ + 'attended → Phase 3p: `/idd-plan`' "$ALL" +assert_grep "idd-all downgrades Plan tier only under unattended" \ + 'unattended → Phase 3a: idd-implement' "$ALL" + +# The reader side. +refute_grep "idd-plan no longer states the blanket 'idd-all must not take the Plan path'" \ + "idd-all 不該走 Plan path**。Plan tier 的核心價值" "$PLAN" +assert_grep "idd-plan splits the two interaction modes" "attended | unattended" "$PLAN" +assert_grep "idd-plan says the downgrade is unattended-only" "降級只發生在 unattended" "$PLAN" +assert_grep "idd-plan defers to idd-all as the normative source" \ + "normative source 是" "$PLAN" + +print_summary "plan-routing-consistency" +exit $? diff --git a/plugins/issue-driven-dev/skills/idd-plan/SKILL.md b/plugins/issue-driven-dev/skills/idd-plan/SKILL.md index 711d3eb..33915e6 100644 --- a/plugins/issue-driven-dev/skills/idd-plan/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-plan/SKILL.md @@ -244,14 +244,14 @@ idd-implement 偵測「已有 `## Implementation Plan` comment 在 issue 上」 ## 與 idd-all 的整合 -`idd-all` Phase 3 的 routing 也會新增 Plan path（v2.36.0+）： +`idd-all` Phase 3 的 routing **依 interaction 軸分流**，Plan tier 的兩種模式走不同路（#292 之後）。normative source 是 [`idd-all/SKILL.md`](../idd-all/SKILL.md) 的 dispatch table；本表與它衝突時以它為準： -``` -| Complexity 值 | Phase 3 行為 | -|--------------|-------------| -| Simple | Phase 3a: idd-implement --pr | -| Plan | Phase 3p: idd-plan --pr (chains to idd-implement --pr internally) | -| Spectra | Phase 3b: spectra-discuss → spectra-propose → spectra-apply (unattended chain) | -``` +| Complexity | attended | unattended | +|---|---|---| +| `Simple` | Phase 3a：`/idd-implement` | Phase 3a：`/idd-implement` | +| `Plan` | **Phase 3p：`/idd-plan`** —— 本 skill 的 `EnterPlanMode` 閘門照常 fire，approve 後由本 skill chain 到 `/idd-implement` | Phase 3a：`/idd-implement` 直送，final report 標記 `[Plan tier deliberation skipped under unattended mode]` | +| `Spectra` | Phase 3b：`spectra-discuss → propose → apply` | 同左 | + +**降級只發生在 unattended。** Plan tier 的價值是 user approval：unattended 沒有 user 在鍵盤前，閘門無從等待，所以那條路徑是**刻意**的降級、而且必須在報告裡講出來。attended 相反 —— 把 plan 呈現給 user 正是 attended 模式存在的理由之一。 -unattended idd-all 模式下，idd-plan 的 EnterPlanMode 會被怎麼處理？— **idd-all 不該走 Plan path**。Plan tier 的核心價值是 user approval，unattended 直接跳過 = 退化成 Simple。idd-all Phase 3 看到 Complexity=Plan 應該 fallback 走 Simple path（idd-implement 直接），並在 final report 標記「Plan tier deliberation skipped under unattended mode」。 +> **本段曾經是一句全稱句**（#317）：「**idd-all 不該走 Plan path**…應該 fallback 走 Simple path」。它的 unattended 半段是對的，錯在把一個分支的結論寫成無條件的 —— 而 #292 之後，attended `idd-all` 走的正是 `/idd-plan`。以本檔為準的讀者會得到與實際行為相反的結論。**能列舉的就列舉**：兩種模式各佔一格，不要先寫一句總括再補例外。 From 23ef4ab817a2df7998b9f52f4faf6c44a63233bd Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:53:40 +0800 Subject: [PATCH 10/11] =?UTF-8?q?docs:=20CHANGELOG=20=E8=A3=9C=20#317=20?= =?UTF-8?q?=E6=A2=9D=E7=9B=AE=20+=20suite=20=E6=95=B8=E6=9B=B4=E6=AD=A3=20?= =?UTF-8?q?49=E2=86=9250?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- plugins/issue-driven-dev/CHANGELOG.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/plugins/issue-driven-dev/CHANGELOG.md b/plugins/issue-driven-dev/CHANGELOG.md index 1ceff35..1016005 100644 --- a/plugins/issue-driven-dev/CHANGELOG.md +++ b/plugins/issue-driven-dev/CHANGELOG.md @@ -66,6 +66,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`idd-clarify`'s worked example** still used the analysis style `#294` replaced with questions, forty lines below the rule and its ❌/✅ demonstration. The example is the half people copy. `scripts/tests/clarify-question-column/` pins it. +- **`idd-plan` described `idd-all`'s Plan routing as a universal** (`#317`) — "idd-all 不該走 Plan path". Its unattended + half was correct; the error was writing one branch's conclusion without its condition, and after `#292` an **attended** + `idd-all` routes to `/idd-plan` precisely. Split into two columns. + `scripts/tests/plan-routing-consistency/` asserts the routing against `idd-all` first — pinning a reader's account + without checking the source would just pin fiction — then asserts the split exists. + ### Honest residue - The `.idd` re-check after `mkdir -p` narrows a TOCTOU window and **has no test weight** — a single-process test @@ -75,7 +81,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `Cannot index string with string ("state")` on jq 1.7 — the value is **not** echoed, so nothing attacker-controlled arrives. Comment corrected, code left alone: a Unicode-aware scrub would mean a second copy of the character class, and divergent copies of a safety definition are this file's longest-running failure. -- Suites: 47 → 49. Classifier assertions: 89 → 120. +- Suites: 47 → 50. Classifier assertions: 89 → 120. ## [2.109.0] - 2026-08-15 From f3e3a758ccacf534746967ac4967bb22d8bab7c9 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 15 Aug 2026 22:58:26 +0800 Subject: [PATCH 11/11] =?UTF-8?q?feat:=20ensemble=20=E7=8F=BE=E5=9C=A8?= =?UTF-8?q?=E6=9C=83=E8=A2=AB=E5=91=8A=E7=9F=A5=20diff=20=E4=B9=8B?= =?UTF-8?q?=E5=A4=96=E7=9A=84=E5=AF=AB=E5=85=A5=EF=BC=88#315=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `idd-verify` 的 scope 是 diff，但 `idd-implement` 的 sister sweep 與 cross-reference note **會寫到 diff 以外的表面**：別的 issue 的 comment、別的 repo 的新 issue。沒有 任何 lens 看得到它們。 實例（`macdoc#143`）：實作筆記裡一個事實錯誤被原樣擴散到另一張 issue 的參照 note。 四個 lens 都只評了 diff 內的措辭，是 devil's advocate **越出自己的 scope** 去讀 Implementation Complete 的 blast radius 紀錄才抓到。**某個 reviewer 臨場越界不是 機制。** 把 implement 自己記下的外部寫入清單塞進 reviewer context，**兩個 backend 都給** —— pai 與 manual fan-out 拿到不同的 context，會讓一個 finding 取決於當時解析到哪個 backend，而 skill 自己的契約說 Step 3 之後兩者可互換。 **沒有紀錄時報 UNKNOWN，不報「沒有外部寫入」**：漏跑的 sister sweep 與跑了但沒找到 的 sister sweep，留下的痕跡一模一樣。 過程中自己踩到一次同型缺陷：第一版的插入點鎖在只有 requirements 那個 prompt 才有 的 attachment 行，於是**五個 prompt 只改到一個**、另外四個靜靜留著舊 context —— 正是這一輪一直在抓的「改了被點到的那行、留下隔壁」。改成鎖每個 prompt 都有的 `Diff path:` 行，並在測試裡**數**：annotated 數必須等於 prompt 數，且 prompt 數 ≥4（防 0 == 0 的空洞相等）。 這是 #315 的**選項 1**。選項 2（idd-implement 寫 machine-readable manifest、 idd-verify 逐筆抽查內容）是**兩個 skill 之間的新契約**，而整輪 audit 的結論正是 「沒經過自己那輪 review 的新契約就是缺陷住的地方」。不做，理由與殘餘風險寫進 CHANGELOG，不留在 PR 討論裡。 新增 `scripts/tests/verify-external-writes/`（第 51 個 suite，7 assertions）。 全 suite 51/51。 --- plugins/issue-driven-dev/CHANGELOG.md | 15 ++++- .../tests/verify-external-writes/test.sh | 61 +++++++++++++++++++ .../skills/idd-verify/SKILL.md | 35 +++++++++++ 3 files changed, 110 insertions(+), 1 deletion(-) create mode 100755 plugins/issue-driven-dev/scripts/tests/verify-external-writes/test.sh diff --git a/plugins/issue-driven-dev/CHANGELOG.md b/plugins/issue-driven-dev/CHANGELOG.md index 1016005..6082d2c 100644 --- a/plugins/issue-driven-dev/CHANGELOG.md +++ b/plugins/issue-driven-dev/CHANGELOG.md @@ -23,6 +23,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 again before posting, and refuses on anything that is not `rc == 0` — including every *uncertain* case, and including "the helper is missing", since a gate you cannot find is not a gate. +- **The ensemble is now told about writes that happen outside the diff** (`#315`). `idd-verify`'s scope is a diff, but + `idd-implement`'s sister sweep and cross-reference notes write to surfaces that are not in it. In the recorded case + (`macdoc#143`) a factual error in an implementation note was propagated verbatim into another issue's cross-reference + note; four lenses reviewed only the wording inside the diff, and the devil's advocate caught it by stepping outside + its brief. One reviewer improvising is not a mechanism. The implementation's own record of external writes now goes + into the reviewer context on **both** backends — giving pai and the manual fan-out different context would make a + finding depend on which backend resolved. An absent record reports the blast radius as **UNKNOWN**, not as empty: + a missing sister-sweep looks exactly like a sweep that found nothing. + + This is `#315`'s option 1. Option 2 — a machine-readable manifest written by `idd-implement` and content-checked by + `idd-verify` — is a new contract between two skills, and the finding of this whole audit is that new contracts + shipped without their own review are where the defects live. Not done; recorded here rather than left implied. + ### Fixed — the rest of the post-merge audit (Codex cross-model pass) - **`migrate-idd-config.sh` only handles ordinary files now.** Four shapes it accepted silently: `.idd` as a symlink @@ -81,7 +94,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `Cannot index string with string ("state")` on jq 1.7 — the value is **not** echoed, so nothing attacker-controlled arrives. Comment corrected, code left alone: a Unicode-aware scrub would mean a second copy of the character class, and divergent copies of a safety definition are this file's longest-running failure. -- Suites: 47 → 50. Classifier assertions: 89 → 120. +- Suites: 47 → 51. Classifier assertions: 89 → 120. ## [2.109.0] - 2026-08-15 diff --git a/plugins/issue-driven-dev/scripts/tests/verify-external-writes/test.sh b/plugins/issue-driven-dev/scripts/tests/verify-external-writes/test.sh new file mode 100755 index 0000000..870e2d4 --- /dev/null +++ b/plugins/issue-driven-dev/scripts/tests/verify-external-writes/test.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# Test: the ensemble is told about writes that happen OUTSIDE the diff (#315). +# +# WHY THIS EXISTS +# +# `idd-verify`'s scope is a diff. But `idd-implement`'s sister sweep and +# cross-reference notes write to surfaces that are NOT in it — comments on other +# issues, issues filed in other repos. No lens can see them. +# +# The recorded case (macdoc#143): a factual error in an implementation note was +# propagated verbatim into a cross-reference note on another issue. Four lenses +# reviewed only the wording inside the diff; the devil's advocate caught it by +# stepping OUTSIDE its scope to read the Implementation Complete comment's blast +# radius. One reviewer improvising past its brief is not a mechanism. +# +# #315 offered three options. This is option 1 — put the implementation's own +# record of external writes into the reviewer context, so they at least know the +# surfaces exist and are asked to check them. Option 2 (a machine-readable +# manifest written by idd-implement and content-checked by idd-verify) is a NEW +# CONTRACT BETWEEN TWO SKILLS, and this session's whole finding is that new +# contracts shipped without their own review are where the defects live. Not +# done here; the residue is recorded in the CHANGELOG. +# +# BOTH BACKENDS, or the context is a coin flip: the same run reviewed through +# pai gets the blast radius and through the manual fan-out does not, which makes +# a finding depend on which backend resolved. The skill's own contract promises +# the two are interchangeable after Step 3. +set -u +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PLUGIN="$(cd "$HERE/../../.." && pwd)" +. "$(cd "$HERE/../../lib" && pwd)/assert-helpers.sh" + +MD=$(cat "$PLUGIN/skills/idd-verify/SKILL.md") + +assert_grep "the external-writes list is collected as a Step 0 task" \ + 'TaskCreate(name="collect_external_writes"' "$MD" +assert_grep "it is read from the Implementation Complete comment" \ + 'Implementation Complete' "$MD" +assert_grep "Tier 1 (pai) receives it through CONTEXT_BLOCK" \ + 'WRITES OUTSIDE THIS DIFF' "$MD" + +# Every manual-fan-out prompt must carry it too. Counting is the point: the +# first attempt at this edit keyed on a line only ONE of the five prompts has, +# so four kept the old context and nothing said so. +PROMPTS=$(printf '%s\n' "$MD" | grep -c 'Diff path: \$VERIFY_DIR/diff\.patch') +ANNOTATED=$(printf '%s\n' "$MD" | grep -c 'Writes OUTSIDE this diff') +assert_eq "every manual-fan-out lens prompt carries the external-writes context" \ + "$PROMPTS" "$ANNOTATED" +require "...and there is more than one of them (guards against a vacuous 0 == 0)" \ + bash -c '[ "$0" -ge 4 ]' "$PROMPTS" + +# The absence case is the one that matters. "No section" means the blast radius +# is UNKNOWN, not that it was empty — an absent record is exactly what a missing +# sister-sweep looks like. +assert_grep "an empty record is reported as UNKNOWN, not as 'nothing happened'" \ + "blast radius as UNKNOWN rather than" "$MD" +assert_grep "...and the pai-side wording says the same" \ + "is simply unknown" "$MD" + +print_summary "verify-external-writes" +exit $? diff --git a/plugins/issue-driven-dev/skills/idd-verify/SKILL.md b/plugins/issue-driven-dev/skills/idd-verify/SKILL.md index 74427f9..3e00eda 100644 --- a/plugins/issue-driven-dev/skills/idd-verify/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-verify/SKILL.md @@ -224,6 +224,30 @@ CONTEXT_BLOCK="${CONTEXT_BLOCK} Source-of-truth attachments (repo-relative; read with your file tools): ${ATTACHMENT_LIST:-（none）}" +# ── diff 之外的寫入（#315）── +# verify 的 scope 是 diff，但 implement 階段的 sister sweep / cross-reference note +# **會寫到 diff 以外的表面**：別的 issue 的 comment、別的 repo 的新 issue。那些內容 +# 沒有任何 lens 看得到。macdoc#143 的實例：實作筆記裡一個事實錯誤被原樣擴散到另一張 +# issue 的參照 note，四個 lens 都只評了文件內的措辭，是 DA **越出 scope** 去讀 +# Implementation Complete 的 blast radius 紀錄才抓到。靠某個 lens 臨場越界不是機制。 +# +# 最小版（#315 選項 1）：把 implement 自己記下的外部寫入清單塞進 context，讓 reviewer +# **知道它們存在**、並被明確要求去核對。這裡不做選項 2 的 machine-readable manifest +# ——那要 idd-implement 與 idd-verify 之間新增一份契約，屬於另一次改動（理由與殘餘 +# 風險見下方）。 +EXTERNAL_WRITES=$(gh issue view "$N" --repo "$GITHUB_REPO" --json comments \ + --jq '[.comments[].body] | map(select(test("(?i)^##+[^\\p{L}]*Implementation Complete"))) | last // ""' 2>/dev/null \ + | awk '/^###[[:space:]]*(Sister Bugs Filed|Blast Radius|Cross-reference|External writes)/{f=1} f && /^###[[:space:]]/ && !/Sister Bugs Filed|Blast Radius|Cross-reference|External writes/{f=0} f') +CONTEXT_BLOCK="${CONTEXT_BLOCK} + +WRITES OUTSIDE THIS DIFF, as recorded by the implementation step. These are real +surfaces the change touched — comments on other issues, issues filed in other +repos — and they are NOT in the diff you are reviewing. Check whether what was +written there is consistent with what the diff actually does: a factual error in +an implementation note propagates to every issue it was cross-referenced into, +and no amount of reading the diff will surface it. +${EXTERNAL_WRITES:-（none recorded — this is not the same as \"none happened\": if the Implementation Complete comment has no such section, the blast radius is simply unknown, and you should say so rather than assume it was empty.）}" + # Tier 1 — canonical：已安裝的 parallel-ai-agents 引擎（#207 使用者依賴裁決；契約 = pai#20 官方化的 EXTERNAL-CONSUMER CONTRACT） MIN_PAI="2.19.0" # codexModel/codexEffort 契約起點（pai#22）——閘門理由：2.18.0 引擎會「靜默忽略」這兩個 args → canonical tier 的 codex 治理斷鏈（#264；同 #205 的 agentModel 教訓：靜默忽略比失敗糟） # PAI_DIR / PAI_VER 已於共通前置解析（#264 重排 —— codex-call 路徑與 engine 路徑兩用） @@ -289,6 +313,7 @@ TaskCreate(name="scan_pr_body_and_commits_trailers", description="Step 0.8: PR m TaskCreate(name="resolve_scratch_dir", description="Step 0.4 (#288): VERIFY_DIR=$(mktemp -d \"${TMPDIR:-/tmp}/idd-verify-${NUMBER}-XXXXXX\") — 一次解析、之後所有 diff / prompt / findings / codex 檔全部掛在它底下。**必須在任何寫檔或 spawn 之前**。固定名稱（舊的 /tmp/verify_${NUMBER}_*）不帶 repo 身分，同一個 issue 號在不同 repo 的兩個 session 會共用檔名，前一輪的殘檔會被當成這一輪的 findings 讀進來 —— 靜默，且方向最壞（把別的 repo 的判決併進這份報告）") TaskCreate(name="get_diff_and_issue", description="依 input source 取 diff（gh pr diff / git diff HEAD~N / git diff origin/...） + gh issue view,存 diff 到 $VERIFY_DIR/diff.patch 供 agents 讀取,並記 FROZEN_SHA=$(git rev-parse HEAD)（PR mode 記 PR head oid — #228 freshness 錨點）；PR mode 額外做 gh pr checkout 並記住原 branch") TaskCreate(name="check_attachments", description="確認 .claude/.idd/attachments/issue-NNN/ 存在,把 attachment 路徑塞進 reviewer agent prompt 作為 source-of-truth context。manifest 缺漏 → 警告繼續(reviewer 仍跑,但 verification 完整度受限)。依 rules/process-attachments.md。") +TaskCreate(name="collect_external_writes", description="#315: 讀最新 ## Implementation Complete comment 的 ### Sister Bugs Filed / Blast Radius / Cross-reference 區段,把 diff 之外的寫入清單塞進 CONTEXT_BLOCK。verify 的 scope 是 diff,但 implement 的 sister sweep / cross-reference note 會寫到別的 issue、別的 repo —— 那些內容沒有任何 lens 看得到。**沒有該區段時要說『blast radius 未知』,不可當成『沒有外部寫入』**") TaskCreate(name="resolve_dispatch_model", description="解析 $AGENT_MODEL — IDD_AGENT_MODEL 未設 → opus；非法值 → abort with usage error（#205；兩個 backend 共用，Workflow args 傳 agentModel、manual 模板填 model）；#264 同步解析 codex 治理（check-plugin-presence.sh codex-pro codex-pro → CP defaults.json + profile.yaml 兩層 → CODEX_MODEL/EFFORT/MAX_TIME，缺席 fail-fast）") TaskCreate(name="launch_parallel_reviewers", description="第一波 5 個 tool calls 同一 message: 4 lens Agent(subagent_type=general-purpose, model=$AGENT_MODEL) for requirements/logic/security/regression + 1 Bash codex(run_in_background:true)；DA 不在此波（#130 sequenced）。prompt 引用 attachment 路徑 + 強制 file-output rule (per #52)") TaskCreate(name="spawn_sequenced_da", description="#130: 4 份 lens findings 檔全部就緒（non-empty）後，coordinator 序列 spawn Devil's Advocate（model=$AGENT_MODEL，prompt 直附 4 檔路徑，無 polling）") @@ -614,6 +639,8 @@ ${BODY} Diff path: $VERIFY_DIR/diff.patch Attachment paths (if any): .claude/.idd/attachments/issue-${NUMBER}/... +Writes OUTSIDE this diff, as recorded by the implementation step (#315) — comments on other issues, issues filed in other repos. Check them against what the diff actually does: a factual error in an implementation note propagates to everything it was cross-referenced into, and reading the diff will never surface it. If nothing is listed below, report the blast radius as UNKNOWN rather than assuming it was empty: +${EXTERNAL_WRITES:-(none recorded)} 你的任務：逐一檢查 issue 的每個要求是否在 code 中被實現。 對每個要求標記：FULLY / PARTIALLY / NOT addressed。 @@ -631,6 +658,8 @@ Agent({ prompt: `你是 Logic Reviewer for Issue #${NUMBER}: ${TITLE}. Diff path: $VERIFY_DIR/diff.patch +Writes OUTSIDE this diff, as recorded by the implementation step (#315) — comments on other issues, issues filed in other repos. Check them against what the diff actually does: a factual error in an implementation note propagates to everything it was cross-referenced into, and reading the diff will never surface it. If nothing is listed below, report the blast radius as UNKNOWN rather than assuming it was empty: +${EXTERNAL_WRITES:-(none recorded)} 你的任務：檢查邏輯正確性。 - Edge cases（null、empty、boundary values） @@ -650,6 +679,8 @@ Agent({ prompt: `你是 Security Reviewer for Issue #${NUMBER}: ${TITLE}. Diff path: $VERIFY_DIR/diff.patch +Writes OUTSIDE this diff, as recorded by the implementation step (#315) — comments on other issues, issues filed in other repos. Check them against what the diff actually does: a factual error in an implementation note propagates to everything it was cross-referenced into, and reading the diff will never surface it. If nothing is listed below, report the blast radius as UNKNOWN rather than assuming it was empty: +${EXTERNAL_WRITES:-(none recorded)} 你的任務：檢查安全問題。 - SQL injection（字串拼接 vs parameterized） @@ -669,6 +700,8 @@ Agent({ prompt: `你是 Regression Reviewer for Issue #${NUMBER}: ${TITLE}. Diff path: $VERIFY_DIR/diff.patch +Writes OUTSIDE this diff, as recorded by the implementation step (#315) — comments on other issues, issues filed in other repos. Check them against what the diff actually does: a factual error in an implementation note propagates to everything it was cross-referenced into, and reading the diff will never surface it. If nothing is listed below, report the blast radius as UNKNOWN rather than assuming it was empty: +${EXTERNAL_WRITES:-(none recorded)} 你的任務： 1. 有沒有改到 issue 範圍外的東西（scope creep）？ @@ -688,6 +721,8 @@ Agent({ prompt: `你是 Devil's Advocate for Issue #${NUMBER}: ${TITLE}. Diff path: $VERIFY_DIR/diff.patch +Writes OUTSIDE this diff, as recorded by the implementation step (#315) — comments on other issues, issues filed in other repos. Check them against what the diff actually does: a factual error in an implementation note propagates to everything it was cross-referenced into, and reading the diff will never surface it. If nothing is listed below, report the blast radius as UNKNOWN rather than assuming it was empty: +${EXTERNAL_WRITES:-(none recorded)} 你是在 4 份 lens findings 檔就緒後才被 spawn 的（coordinator 已確認 — #130 sequenced 模式，無需 polling）。直接讀取 4 份 sibling findings，然後：