From 1e466981bd987bcc26ce43dbe632f87ffb19c7f7 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 26 Jul 2026 09:10:01 +0900 Subject: [PATCH] fix(skills): name the diagnosis precondition in idd-plan's description (#276) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A skill's frontmatter `description` is the only surface an AI reads when choosing which skill to invoke — the SKILL.md body loads only after the skill has already been selected. idd-plan's description named no precondition, so an AI advised a user to skip `/idd-diagnose` and run `/idd-plan` directly; that inference was correct on the information visible to it. The hard abort at skills/idd-plan/SKILL.md:62 stays exactly as-is (implementation is authoritative) — this corrects the machine-facing surface only. Measuring all 17 skills against the house pattern (`Use when:` + `防止的失敗：`) found 5 off it, not 1 as the diagnosis predicted. All 5 are corrected together per the shared-abstraction contract: idd-plan precondition + remedy + failure mode (root cause) idd-clarify Use when + failure mode idd-comment Use when idd-all-chain failure mode idd-report failure mode idd-plan and idd-clarify were also the only two skills carrying a bare single-line description rather than a `description: |` block — the format itself acted as an affordance for under-specification. Both now use the block form. New drift-guard suite scripts/tests/skill-description-contract asserts A1 field present, A2 `Use when`, A3 `防止的失敗`, A4 diagnosis precondition for an explicit allowlist (idd-plan, idd-implement). Auto-discovered by run-all-tests.sh. RED 8 failing -> GREEN 56/0; reverting idd-plan alone re-reds exactly 3, so the assertions are not placebos. Aggregator: 41 suites, 0 failed. Phase 1 of two. The wiki flowchart generated from docs/workflows.md is deliberately not in this change, and the issue stays open. --- plugins/issue-driven-dev/CHANGELOG.md | 19 ++++ plugins/issue-driven-dev/README.md | 2 +- .../tests/skill-description-contract/test.sh | 106 ++++++++++++++++++ .../skills/idd-all-chain/SKILL.md | 1 + .../skills/idd-clarify/SKILL.md | 6 +- .../skills/idd-comment/SKILL.md | 2 +- .../issue-driven-dev/skills/idd-plan/SKILL.md | 6 +- .../skills/idd-report/SKILL.md | 1 + 8 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 plugins/issue-driven-dev/scripts/tests/skill-description-contract/test.sh diff --git a/plugins/issue-driven-dev/CHANGELOG.md b/plugins/issue-driven-dev/CHANGELOG.md index 3705974..8b5fc7b 100644 --- a/plugins/issue-driven-dev/CHANGELOG.md +++ b/plugins/issue-driven-dev/CHANGELOG.md @@ -5,6 +5,25 @@ 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). +## [Unreleased] + +### Fixed + +- **`idd-plan` advertised no diagnosis precondition, so AIs routed users around `idd-diagnose` (#276, Phase 1)** — a user's AI told them to skip `/idd-diagnose` and run `/idd-plan` directly; following that advice hits `idd-plan`'s hard abort. The advice was *correct on the information the AI could see*. The plan↔diagnose ordering lived on three surfaces, and the only one readable at routing time was silent about it: `docs/workflows.md` `P-plan-gated` (never auto-loaded), `skills/idd-plan/SKILL.md` Step 0 hard abort (loads only *after* the skill is already chosen), and the frontmatter `description` — the sole input to the choice — which named no precondition and positioned the skill purely relative to `idd-implement`. Combined with an effort-minimizing read of "plan" as a superset of "diagnose", the mis-route was structural, not incidental. `idd-plan`'s description now states the precondition **and the remedy** (`run /idd-diagnose #N first`), so a mis-routing AI is handed the correct next step rather than merely a warning it will learn to avoid. `README.md`'s skill row was reworded off the tier-spectrum vocabulary ("sits between Simple and Spectra") onto user vocabulary (when you reach it, what happens if you don't). The hard abort itself is unchanged — implementation stays the source of truth; this closes the docs/metadata gap around it. +- **Four sibling skills off the frontmatter house pattern** — measurement during planning found the defect was family-wide, not a one-file slip (diagnosis had predicted one): `idd-clarify` (no `Use when` / `防止的失敗`), `idd-comment` (no `Use when` — it had the semantic equivalent, "用於…時", but not at the position an AI scans), `idd-all-chain` and `idd-report` (no `防止的失敗`). All five are fixed together per the #129 shared-abstraction contract and the #44 lesson that fixing only the triggering member costs a second close. + +### Added + +- **`skill-description-contract` drift-guard suite** — the forcing function the convention never had. Iterates every `skills/*/` and asserts, per skill: the frontmatter carries a non-empty `description` (A1), which states when to reach for the skill (A2, `Use when`) and what failure it prevents (A3, `防止的失敗`); skills in an explicit `DIAGNOSIS_DEPENDENT` allowlist must additionally name the diagnosis precondition (A4). New skills are covered by construction — the glob picks them up, so shipping one without the house pattern is immediately RED. Two limits are documented in the suite header rather than papered over: it asserts **presence, not quality** (`Use when: 需要時` would pass — prose depth stays review's job, same stance as `docs-catalog-sync`), and A4 is an explicit list rather than derived, because the obvious derivation ("any skill whose body tells the user to run `/idd-diagnose` first") false-positives on `idd-close`, where that string is a meeting-deliverable fallback and not close's own precondition (close gates on verify). + +### Tests + +- New suite `skill-description-contract`, 56 assertions. TDD-verified both directions: RED at exactly 8 failures against the pre-fix tree (`idd-plan`×3, `idd-clarify`×2, `idd-comment`/`idd-all-chain`/`idd-report`×1 each), GREEN 56/0 after; anti-placebo pass reverted `idd-plan` alone and confirmed exactly its 3 assertions flip back while the other 53 hold (the #119 placebo-gate failure class). The allowlist also self-guards — a renamed or deleted `DIAGNOSIS_DEPENDENT` entry fails loudly instead of silently asserting nothing. Aggregator 41 suites, 0 fail. + +### Notes + +- #276 stays **open**: this is Phase 1 only. Phase 2 — generating a wiki workflow flowchart from `docs/workflows.md` so the 31 paths are legible without reading the catalog — is deliberately unbundled. It has three open design questions (parse contract, push mechanism, how to render 31 paths) and writes to a second git repo, so binding the root-cause fix to it would leave the mis-routing shipping meanwhile. + ## [2.101.0] - 2026-07-19 ### Added diff --git a/plugins/issue-driven-dev/README.md b/plugins/issue-driven-dev/README.md index f3761f0..69950a0 100644 --- a/plugins/issue-driven-dev/README.md +++ b/plugins/issue-driven-dev/README.md @@ -57,7 +57,7 @@ idd-issue → idd-diagnose → idd-implement → idd-verify → idd-close |-------|---------| | `idd-issue` | Create well-documented GitHub Issue with original quotes and images | | `idd-diagnose` | Find root cause (bug) or analyze requirements (feature/refactor) | -| `idd-plan` | Plan tier approval gate using `EnterPlanMode` — presents Implementation Plan for user approval before TDD execution; sits between `Simple` direct-implement and `Spectra` spec-contract path (v2.36.0+) | +| `idd-plan` | Approval gate before TDD — presents the Implementation Plan via `EnterPlanMode` and waits for your OK. **Runs after `idd-diagnose`, never instead of it**: you reach it when the diagnosis routes the issue to Plan tier (the middle ground between `Simple` direct-implement and the full `Spectra` spec path). Invoking it on an undiagnosed issue aborts — run `/idd-diagnose #N` first (v2.36.0+) | | `idd-implement` | Scope-disciplined implementation with TDD | | `idd-verify` | Independent verification — 5 Claude reviewer agents + cross-model Codex leg (gpt-5.x; executable from pai, governance from codex-pro per #264). `--profile code/prose/academic` + `--file`/`--dir` for non-code deliverables (#258, v2.97+) | | `idd-close` | Closing comment documenting problem, root cause, solution, verification | diff --git a/plugins/issue-driven-dev/scripts/tests/skill-description-contract/test.sh b/plugins/issue-driven-dev/scripts/tests/skill-description-contract/test.sh new file mode 100644 index 0000000..c601aeb --- /dev/null +++ b/plugins/issue-driven-dev/scripts/tests/skill-description-contract/test.sh @@ -0,0 +1,106 @@ +#!/usr/bin/env bash +# test.sh — drift-guard for the skill frontmatter description contract (#276). +# +# ROOT CAUSE THIS GUARDS: a skill's frontmatter `description` is the ONLY surface +# an AI reads when deciding which skill to invoke. Everything else — SKILL.md body, +# docs/workflows.md path catalog, README — is either loaded too late (after the +# skill was already chosen) or never auto-loaded at all. In #276 an AI advised a +# user to skip `/idd-diagnose` and run `/idd-plan` directly; that inference was +# CORRECT on the information visible to it, because idd-plan's description named +# no precondition. The routing error was structural, not incidental. +# +# Contract, per skill under plugins/issue-driven-dev/skills/: +# A1 frontmatter exists and carries a non-empty `description` +# A2 the description states WHEN to reach for the skill (`Use when`) +# A3 the description states WHAT FAILURE it prevents (`防止的失敗`) +# A4 skills in DIAGNOSIS_DEPENDENT name the diagnosis precondition (`diagnos`) +# +# The guard asserts PRESENCE of these signals at a predictable position, NOT their +# quality — someone can still satisfy A2 with "Use when: 需要時". Prose quality stays +# human judgment at review. Same self-declared stance as scripts/tests/docs-catalog-sync +# ("asserts MENTION, not quality"): a mechanical guard buys consistency of surface, +# never depth of content. +# +# WHY A4 IS AN EXPLICIT ALLOWLIST AND NOT DERIVED: the obvious derivation — +# "any skill whose body tells the user to run /idd-diagnose first" — was tried and +# rejected. `grep -l '先跑 `/idd-diagnose'` also matches skills/idd-close/SKILL.md, +# where the string sits in a meeting-deliverable fallback ("neither deliverable +# present → send them back to diagnose"), NOT as idd-close's own precondition +# (close is gated on verify). A derived rule would force idd-close to advertise a +# precondition it does not have. The IDD lifecycle graph is small and stable, so +# an explicit list costs less to maintain than a misfiring inference costs to debug. +# +# Usage: bash test.sh (exit 0 = all pass, 1 = any fail) + +set -u + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PLUGIN_ROOT="$HERE/../../.." +SKILLS_DIR="$PLUGIN_ROOT/skills" + +HELPERS="$HERE/../../lib/assert-helpers.sh" +[ -f "$HELPERS" ] || { echo "✗ missing $HELPERS — cannot run suite" >&2; exit 1; } +. "$HELPERS" + +# Skills whose description MUST name the diagnosis precondition. See header for +# why this is enumerated rather than inferred. +DIAGNOSIS_DEPENDENT=(idd-plan idd-implement) + +# Emit the `description` field's full value (block scalar or single line): +# everything from the `description:` line until the next top-level frontmatter key. +extract_description() { # file + awk ' + /^---[[:space:]]*$/ { fence++; if (fence >= 2) exit; next } + fence != 1 { next } + /^description:/ { indesc = 1; print; next } + indesc && /^[A-Za-z_][A-Za-z0-9_-]*:/ { indesc = 0 } + indesc { print } + ' "$1" +} + +in_allowlist() { # name + local n="$1" s + for s in "${DIAGNOSIS_DEPENDENT[@]}"; do [ "$s" = "$n" ] && return 0; done + return 1 +} + +assert_file_exists "skills/ directory exists" "$SKILLS_DIR" + +for d in "$SKILLS_DIR"/*/; do + n=$(basename "$d") + f="$d/SKILL.md" + + if [ ! -f "$f" ]; then + fail "A1 $n: SKILL.md exists" "no SKILL.md under skills/$n/" + continue + fi + + desc=$(extract_description "$f") + + # A1 — the field itself. + if printf '%s\n' "$desc" | grep -qE '^description:[[:space:]]*\S'; then + pass "A1 $n: frontmatter has a description" + else + fail "A1 $n: frontmatter has a description" \ + "skills/$n/SKILL.md has no non-empty 'description:' in its YAML frontmatter" + fi + + # A2 — when to reach for it. + assert_grep "A2 $n: description says 'Use when'" "Use when" "$desc" + + # A3 — what failure it prevents. + assert_grep "A3 $n: description says '防止的失敗'" "防止的失敗" "$desc" + + # A4 — diagnosis precondition, allowlisted skills only. + if in_allowlist "$n"; then + assert_grep_re "A4 $n: description names the diagnosis precondition" "diagnos" "$desc" + fi +done + +# Placebo guard on the allowlist itself: a renamed or deleted skill must not let +# A4 silently stop asserting anything (the #119 placebo-gate failure class). +for s in "${DIAGNOSIS_DEPENDENT[@]}"; do + assert_file_exists "A4 allowlist entry resolves to a real skill: $s" "$SKILLS_DIR/$s/SKILL.md" +done + +print_summary "skill-description-contract" diff --git a/plugins/issue-driven-dev/skills/idd-all-chain/SKILL.md b/plugins/issue-driven-dev/skills/idd-all-chain/SKILL.md index 0e99102..4e8bf76 100644 --- a/plugins/issue-driven-dev/skills/idd-all-chain/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-all-chain/SKILL.md @@ -5,6 +5,7 @@ description: | Recursive shell over /idd-all — sub-skill spawns (sister bug / follow-up finding / tangential / sister concern) detected via spawn manifest, chain-eligible enqueued automatically. Use when: root issue likely ripples (refactor with sister bugs / spec change with cross-spec impact / multi-layer feature) and you want single PR review. Stops at verified — never auto-close, /idd-close per issue still required. + 防止的失敗：root issue 修完就收工，過程中冒出來的 sister bug / follow-up 散成孤兒 issue，沒人記得要一起 review。 argument-hint: "[#NNN ...] [--bfs] [--review] [--cwd /path/to/clone] e.g. '#28', '#A #B #C --bfs', '#28 --review' (--review opt-in re-opens NSQL confirmation loop at terminal report)" allowed-tools: - Bash(gh:*) diff --git a/plugins/issue-driven-dev/skills/idd-clarify/SKILL.md b/plugins/issue-driven-dev/skills/idd-clarify/SKILL.md index b1dc5ef..34fd12e 100644 --- a/plugins/issue-driven-dev/skills/idd-clarify/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-clarify/SKILL.md @@ -1,6 +1,10 @@ --- name: idd-clarify -description: Scan an existing GitHub issue body for terminology / ambiguity / missing-context gaps and annotate via surfacing-only `### Clarity Surface` block. Standalone primitive — also delegated by `idd-issue` Step 4.6, gated by `idd-diagnose` Step 0.5. +description: | + Scan an existing GitHub issue body for terminology / ambiguity / missing-context gaps and annotate via surfacing-only `### Clarity Surface` block. + Standalone primitive — also delegated by `idd-issue` Step 4.6, gated by `idd-diagnose` Step 0.5. + Use when: issue 已經存在但讀起來含糊 —— 術語沒定義、指涉不明、前提缺失，想在往下走之前先把不清楚的地方標出來讓人補。 + 防止的失敗：帶著沒對齊的假設一路做到底，交付後才發現雙方講的根本不是同一件事。 --- # /idd-clarify — 語意校正 surface diff --git a/plugins/issue-driven-dev/skills/idd-comment/SKILL.md b/plugins/issue-driven-dev/skills/idd-comment/SKILL.md index 407aec0..f1862ff 100644 --- a/plugins/issue-driven-dev/skills/idd-comment/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-comment/SKILL.md @@ -3,7 +3,7 @@ name: idd-comment description: | 加 template-guided comment 到 GitHub issue。 支援 decision/note/question/correction/link/errata/reply 7 個 types，強制 blockquote 原文、加 timestamp 與 metadata marker。 - 用於記錄使用者決定、外部 context、開放問題，不走完整 diagnose/implement phase 時； + Use when: 要記錄使用者決定、外部 context、開放問題，但這件事不值得走完整 diagnose/implement phase 時； reply type（#269）是 human-facing 逐點回覆——寫給 review 提出者等人類 collaborator 的 correspondence。 支援 batch mode（v2.34.0+）：多個 #N 同步加同一段 comment（如 `#34 #36 --type note --body 'blocked by upstream'`）。 防止的失敗：非流程性 decision 散落在 chat，未來無法追溯。 diff --git a/plugins/issue-driven-dev/skills/idd-plan/SKILL.md b/plugins/issue-driven-dev/skills/idd-plan/SKILL.md index 201cae5..711d3eb 100644 --- a/plugins/issue-driven-dev/skills/idd-plan/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-plan/SKILL.md @@ -1,6 +1,10 @@ --- name: idd-plan -description: Plan-mode 實作。在 idd-implement 的 TDD loop 之前，先用 Claude Plan Mode 把 Implementation Plan 呈現給使用者審查、approve 後才動手。介於 Simple（直接 implement）和 Spectra（完整 spec/proposal/tasks artifacts）之間的中間層。 +description: | + Plan-mode 實作。在 idd-implement 的 TDD loop 之前，先用 Claude Plan Mode 把 Implementation Plan 呈現給使用者審查、approve 後才動手。 + 介於 Simple（直接 implement）和 Spectra（完整 spec/proposal/tasks artifacts）之間的中間層。 + Use when: 已跑過 `/idd-diagnose #N`、且該 diagnosis 的 Complexity 判為 Plan tier 之後。本 skill 不做診斷 —— issue 上沒有 Diagnosis comment 會直接 abort，此時該跑的是 `/idd-diagnose #N` 而不是本 skill。 + 防止的失敗：跳過 diagnose 直接 plan —— 還沒查出 root cause 就開始寫實作計畫，等於為錯的問題做規劃。 --- # /idd-plan — Plan-mode 實作 diff --git a/plugins/issue-driven-dev/skills/idd-report/SKILL.md b/plugins/issue-driven-dev/skills/idd-report/SKILL.md index faeb705..2d1633d 100644 --- a/plugins/issue-driven-dev/skills/idd-report/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-report/SKILL.md @@ -4,6 +4,7 @@ description: | 產出進度報告到 GitHub Discussions。 可指定 issue 清單、來源檔案、或 milestone，彙整所有相關 issue 的處理狀態。 Use when: milestone 完成、Sprint review、向客戶/主管匯報進度。 + 防止的失敗：進度只散落在各 issue 的流水帳裡，要匯報時臨時人工翻一遍，還漏掉幾條。 argument-hint: "#157 #158 ... 或 source:檔案名 或 milestone:名稱 [@tag1 @tag2]" allowed-tools: - Bash(gh:*)