From 0c7431aa3e2c8d99e7993c1666f02ad54356fd77 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 10 May 2026 16:39:24 +0800 Subject: [PATCH 1/6] docs(distribution-detection): add canonical contract for distribution detection (Refs #45) NEW reference doc covering: - Detection helpers (is_plugin_marketplace_member, has_binary_wrapper, infer_distribution_type) - D3 mixed-type superset rule (chain plugin-update only, contingent on #66 audit) - Skill resolution table - Escape hatch (IDD_DISTRIBUTION_SYNC_PROMPT=false env var) - Extension protocol for unlisted distribution patterns - Smoke matrix fixtures (manual until #62 test infra) Per Plan tier #45 D1-D5 decisions. SKILL.md Step 6.5 in next commit will delegate to this contract. --- .../references/distribution-detection.md | 189 ++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 plugins/issue-driven-dev/references/distribution-detection.md diff --git a/plugins/issue-driven-dev/references/distribution-detection.md b/plugins/issue-driven-dev/references/distribution-detection.md new file mode 100644 index 0000000..992f662 --- /dev/null +++ b/plugins/issue-driven-dev/references/distribution-detection.md @@ -0,0 +1,189 @@ +# Distribution Detection Contract + +> Canonical contract for detection helpers used by `/idd-close` Step 6.5 (Distribution Sync chain) and any future skill that needs to know whether a repo ships its changes to a user-facing distribution channel. + +**Source**: `idd-close-distribution-sync` change for issue #45 — close-tier checkpoint that complements `che-claude-config/rules/common-release-flow.md` (release-tier). + +## Purpose + +For repos whose fixes reach users via plugin marketplace / MCP binary / CLI binary distribution, IDD lifecycle close has a one-more-mile dropped step: bumping `marketplace.json` or invoking `mcp-deploy`. Without a mechanical checkpoint at close time, "fixed but not shipped" anti-pattern recurs. + +This contract specifies how `/idd-close` Step 6.5 detects whether the closing repo is distributed and which sync skill to chain to. Detection is **read-only** — no state changes, no network calls beyond `gh repo view` style metadata. + +## Distribution types + +| Type | Detection signal | Sync skill | +|------|-----------------|-----------| +| `plugin` | Repo is referenced as `plugins[].source.git` in some ancestor `.claude-plugin/marketplace.json` | `/plugin-tools:plugin-update ` | +| `mcp` | `$REPO_ROOT/bin/*.sh` contains `gh release download` or `curl.*github.com.*releases` AND wrapper script name matches `*-mcp-wrapper.sh` (or contains `mcp` substring) | `/mcp-tools:mcp-deploy` | +| `cli` | Same wrapper detection as MCP, but wrapper script name does NOT match MCP heuristic | `/cli-tools:cli-deploy` | +| `plugin+mcp` / `plugin+cli` | Both signals trigger | `/plugin-tools:plugin-update ` (per D3 superset rule, see below) | +| `n/a` | None of the above | — silent skip | + +## Detection helpers + +These are intended to be implemented inline in `idd-close/SKILL.md` Step 6.5 bash, but documented here as the canonical algorithm for any future caller. + +### `is_plugin_marketplace_member(repo_root, github_repo) -> bool` + +Walk-up parents from `$REPO_ROOT` looking for `.claude-plugin/marketplace.json`, **stop at `$HOME`** (same convention as `find_idd_config` in `references/config-protocol.md`). + +```bash +is_plugin_marketplace_member() { + local repo_root="$1" + local github_repo="$2" # owner/repo form + local current="$repo_root" + + while [ "$current" != "$HOME" ] && [ "$current" != "/" ]; do + local manifest="$current/.claude-plugin/marketplace.json" + if [ -f "$manifest" ]; then + # Check if any plugins[].source.git matches our repo + # Normalize trailing .git for comparison + local match=$(jq -r --arg repo "$github_repo" \ + '.plugins[]?.source.git // empty + | sub(".git$"; "") + | select(endswith($repo))' \ + "$manifest" 2>/dev/null | head -1) + if [ -n "$match" ]; then + echo "true" + return 0 + fi + fi + current=$(dirname "$current") + done + echo "false" +} +``` + +**Stop conditions**: walking past `$HOME` is a strong signal we've left the user's project tree;`/` is a safety bound. Marketplace repos are by convention under `$HOME/Developer/` or similar. + +**Match logic**: `jq` extracts `plugins[].source.git` URLs (e.g. `https://github.com/PsychQuant/issue-driven-development.git`), strips trailing `.git`, then checks if any URL ends with the `owner/repo` form. This is robust to https vs ssh URLs and trailing `.git` variations. + +### `has_binary_wrapper(repo_root) -> bool` + +Scan `$REPO_ROOT/bin/*.sh` (literal pattern, not recursive) for known distribution patterns. + +```bash +has_binary_wrapper() { + local repo_root="$1" + local bin_dir="$repo_root/bin" + + [ -d "$bin_dir" ] || { echo "false"; return; } + + local match=$(ls "$bin_dir"/*.sh 2>/dev/null \ + | xargs grep -l -E 'gh release download|curl.*github\.com.*releases' 2>/dev/null \ + | head -1) + [ -n "$match" ] && echo "true" || echo "false" +} +``` + +**Why `bin/*.sh` only**: build/dev scripts typically live under `scripts/` or `Sources/`;user-facing distribution wrappers conventionally live under `bin/`. False-positive on a non-distribution `bin/*.sh` is covered by AskUserQuestion `not applicable` opt-out (see Step 6.5 prose). + +**Detection keywords**: limited to GitHub-native distribution per #45 issue body scope. Future extension covered below. + +### `infer_distribution_type(repo_root, github_repo) -> string` + +Orchestrator returning one of: `plugin` / `mcp` / `cli` / `plugin+mcp` / `plugin+cli` / `n/a`. + +```bash +infer_distribution_type() { + local repo_root="$1" + local github_repo="$2" + + local is_plugin=$(is_plugin_marketplace_member "$repo_root" "$github_repo") + local has_wrapper=$(has_binary_wrapper "$repo_root") + + local binary_kind="" + if [ "$has_wrapper" = "true" ]; then + # MCP vs CLI heuristic via wrapper script name + local mcp_match=$(ls "$repo_root/bin"/*.sh 2>/dev/null \ + | grep -E '(-mcp-wrapper\.sh$|/[^/]*mcp[^/]*\.sh$)' | head -1) + if [ -n "$mcp_match" ]; then + binary_kind="mcp" + else + binary_kind="cli" + fi + fi + + if [ "$is_plugin" = "true" ] && [ -n "$binary_kind" ]; then + echo "plugin+$binary_kind" + elif [ "$is_plugin" = "true" ]; then + echo "plugin" + elif [ -n "$binary_kind" ]; then + echo "$binary_kind" + else + echo "n/a" + fi +} +``` + +## D3 — Mixed-type superset rule + +When `infer_distribution_type` returns `plugin+mcp` or `plugin+cli`, **chain `plugin-update` only**. + +**Rationale**: per `che-claude-config/rules/common-plugins.md`, `plugin-tools:plugin-update` v1.11+ is **dependency-aware orchestrator** — its Phase 1.5 detects `.mcp.json` / wrapper / session-start hook dependencies and AskUserQuestion-prompts the cascade ("順便更新 binary?" → invokes `mcp-deploy` / `cli-deploy`). Calling `plugin-update` is the superset action;calling both separately would either duplicate work or skip the plugin shell sync. + +**Caveat**: this claim is **unverified empirically** (filed as PsychQuant/issue-driven-development#66 audit). If audit reveals stale claim, this contract should change to "chain both with explicit ordering" — `mcp-deploy` first to bump binary, then `plugin-update` to bump plugin shell pointing at new binary version. + +## Skill resolution table + +| `infer_distribution_type` returns | Sync skill to chain | +|----------------------------------|--------------------| +| `plugin` | `/plugin-tools:plugin-update ` | +| `mcp` | `/mcp-tools:mcp-deploy` | +| `cli` | `/cli-tools:cli-deploy` | +| `plugin+mcp` | `/plugin-tools:plugin-update ` (D3) | +| `plugin+cli` | `/plugin-tools:plugin-update ` (D3) | +| `n/a` | — silent skip, no chain | + +**Plugin name resolution** (when `` is needed): parse the matched `marketplace.json` entry — its `name` field IS the plugin name to pass to `plugin-update`. + +## Caller skill `--source-issue` flag + +`/idd-close` Step 6.5 v1 invokes target skill **without** `--source-issue` flag — caller skills (`plugin-update` / `mcp-deploy` / `cli-deploy`) in `psychquant-claude-plugins` repo don't yet support it (deferred to sister issue per #45 Step 4.7 scope clarification). Graceful: target skills ignore unknown flags. + +When sister issue lands flag support, this contract gains a v2 invocation form: `/plugin-tools:plugin-update --source-issue $NUMBER` so the resulting marketplace commit message includes the source issue ref for traceability. + +## Escape hatch / rollback + +Env var `IDD_DISTRIBUTION_SYNC_PROMPT=false` silently skips Step 6.5 prompt entirely (still 1-line audit `Distribution sync prompt skipped (IDD_DISTRIBUTION_SYNC_PROMPT=false)` in closing comment for traceability). + +**Detection-based silent skip** (non-distribution repo) is **always-on** — env var only suppresses the prompt for distribution-detected repos. + +Rationale: CI / batch runs need disable to avoid AskUserQuestion blocking. Pattern-symmetric with IC_R011 `AI_LOW_BAR_ISSUE_FILING=false` rollback (see `references/ic-r011-checkpoint.md` §5). + +## Extension protocol — adding distribution patterns + +If a user's wrapper uses unlisted keyword (e.g. `wget`, `npm install`, `brew install`): + +1. **Don't edit `idd-close/SKILL.md`** — the SKILL.md delegates to this contract, not inline keywords +2. Edit `has_binary_wrapper` regex in this doc + the inline implementation in `idd-close/SKILL.md` Step 6.5 bash to add the pattern +3. Document the new pattern in this contract under "Detection keywords" section +4. Smoke-test against the repo that has the unusual wrapper + +**Rationale**: encapsulating detection logic here lets future extension touch one canonical doc rather than hunting through skill prose. + +## Test fixtures (manual smoke matrix) + +(No automated test framework yet — covered by PsychQuant/issue-driven-development#62 follow-up.) + +| # | Repo | Setup | `infer_distribution_type` returns | +|---|------|-------|-----------------------------------| +| 1 | `issue-driven-dev` plugin via marketplace | `cd ~/Developer/issue-driven-development;` `marketplace.json` ancestor at `~/Developer/psychquant-claude-plugins/.claude-plugin/marketplace.json` lists this repo | `plugin` | +| 2 | Pure MCP binary | `cd ~/Developer/some-mcp;` `bin/some-mcp-wrapper.sh` contains `gh release download` | `mcp` | +| 3 | Pure CLI binary | `cd ~/Developer/some-cli;` `bin/install.sh` contains `curl https://github.com/.../releases/download/...` | `cli` | +| 4 | Mixed plugin + MCP (`che-zotero-mcp`) | both signals | `plugin+mcp` | +| 5 | Non-distribution (this repo without marketplace context) | no `bin/*.sh` matching, no marketplace.json ancestor | `n/a` | + +## Cross-references + +- `idd-close/SKILL.md` Step 6.5 — primary consumer of this contract +- `che-claude-config/rules/common-release-flow.md` — complementary release-tier trigger (post-release marketplace sync) +- `references/ic-r011-checkpoint.md` — canonical 3-option AskUserQuestion pattern that Step 6.5 conforms to +- `references/config-protocol.md` `find_idd_config` — same walk-up convention used by `is_plugin_marketplace_member` +- PsychQuant/issue-driven-development#66 — audit of `common-plugins.md` plugin-update Phase 1.5 cascade claim that D3 superset rule depends on +- PsychQuant/issue-driven-development#62 — test infra follow-up (will eventually validate the smoke matrix above) + +## Versioning + +This contract is **v1**. Breaking changes (e.g. adding required fields to `infer_distribution_type` output) increment to v2 and require coordinated update of all consumers (currently only `idd-close/SKILL.md`). From 4765092a34fec568803be199b885e1684d946d22 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 10 May 2026 16:41:02 +0800 Subject: [PATCH 2/6] feat(idd-close): add Step 6.5 distribution sync chain detection (Refs #45) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inserts Step 6.5 between Step 6 (auto-update phase=closed) and Step 7 (batch close): - Detection delegated to references/distribution-detection.md canonical contract - Silent skip path for non-distribution repos + IDD_DISTRIBUTION_SYNC_PROMPT=false env var - AskUserQuestion 3-option as prose (per #47 P1 finding 2 — agent-level tool, not bash function) - D3 superset table: plugin+mcp/cli → chain plugin-update only (Phase 1.5 cascade) - Audit trail PATCH appends ### Distribution Sync section to closing comment regardless of choice - Failure-mode summary covers gh api / parse / network failures Step 0.5 Bootstrap Task List adds distribution_sync_chain_detection entry between auto_update_body and report_result (per skill bootstrap rule: every step listed in skill body MUST have a TaskCreate entry). Per Plan tier #45 D1-D5 decisions: - D1 walk-up to $HOME (find_idd_config convention) - D2 keywords: gh release download / curl github releases (extension hook in ref doc) - D3 mixed-type → plugin-update only (contingent on #66 audit) - D4 v1 no --source-issue flag (graceful, sister issue tracks caller-side) - D5 IDD_DISTRIBUTION_SYNC_PROMPT=false rollback hatch --- .../skills/idd-close/SKILL.md | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/plugins/issue-driven-dev/skills/idd-close/SKILL.md b/plugins/issue-driven-dev/skills/idd-close/SKILL.md index 5e06828..fb0311b 100644 --- a/plugins/issue-driven-dev/skills/idd-close/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-close/SKILL.md @@ -165,6 +165,7 @@ TaskCreate(name="review_with_user", description="顯示 closing comment 給使 TaskCreate(name="closing_followup_keyword_scan", description="Step 3.5: scan drafted closing summary for trigger phrases (follow-up / deferred / future / 之後 / 順便 etc); orphan mentions without #NNN cross-link → AskUserQuestion 3-option per canonical references/ic-r011-checkpoint.md; PATCH closing summary inline + add `### Closing Follow-ups Filed` audit trail (advisory, non-blocking, per IC_R011 #527)") TaskCreate(name="publish_and_close", description="gh issue comment + gh issue close") TaskCreate(name="auto_update_body", description="跑 /idd-update #NNN 把 issue body 的 Current Status phase 改 closed（Step 6，常被漏）") +TaskCreate(name="distribution_sync_chain_detection", description="Step 6.5 (v2.56.0+, #45): infer_distribution_type detection per references/distribution-detection.md; if hit → AskUserQuestion 3-option (chain to plugin-update/mcp-deploy/cli-deploy / skip — manual later / not applicable); patch closing comment with ### Distribution Sync section. Silent skip for non-distribution repos. IDD_DISTRIBUTION_SYNC_PROMPT=false env var bypasses prompt entirely (still 1-line audit).") TaskCreate(name="report_result", description="輸出關閉後的 issue URL 與 commits chain") ``` @@ -468,6 +469,133 @@ Skill(skill="issue-driven-dev:idd-update", args="#NNN") **批次 close 時**：每一個 issue 都要分別跑 idd-update，不是跑一次。 +### Step 6.5: Distribution Sync chain (v2.56.0+, #45) + +**Compliance**: this step implements close-tier distribution-sync checkpoint per IC_R011-style 3-option AskUserQuestion + audit trail pattern (canonical reference: [`references/ic-r011-checkpoint.md`](../../references/ic-r011-checkpoint.md)). Complements `che-claude-config/rules/common-release-flow.md` (release-tier trigger). + +**Why this step**: for plugin/MCP/CLI repos, fix lands in main but user-facing distribution channel (marketplace.json / binary release) needs separate sync. Without Step 6.5 mechanical checkpoint, "marketplace.json sync" relies on user memory → 「修了卻沒修」 anti-pattern (e.g. `che-apple-mail-mcp#72` PR #75 merged but marketplace.json not bumped → users `/plugin update` got old binary). + +**Rule**: Step 6 (auto-update phase=closed) completes → Step 6.5 runs detection. If detection returns `n/a` → silent skip + 1-line audit (`(detection: not a distribution repo)`). Otherwise AskUserQuestion 3-option per IC_R011 canonical pattern. + +**Placement rationale**: phase=closed locks audit at Step 6 (lifecycle gate complete);Step 6.5 is follow-up action not lifecycle gate. The closing comment's `### Distribution Sync` section heading clearly separates from main summary so audit trail timeline is unambiguous. + +#### Detection (bash) + +Per [`references/distribution-detection.md`](../../references/distribution-detection.md) canonical contract — implement helpers inline (or source the contract's algorithm): + +```bash +# Detection delegated to contract (algorithm fully spec'd in distribution-detection.md) +DISTRIBUTION_TYPE=$(infer_distribution_type "$REPO_ROOT" "$GITHUB_REPO") +# Returns: plugin / mcp / cli / plugin+mcp / plugin+cli / n/a + +# Silent skip: non-distribution repo +if [ "$DISTRIBUTION_TYPE" = "n/a" ]; then + AUDIT_LINE="### Distribution Sync + +- **Detected**: not a distribution repo (skipped) +- **At**: $(date -u +%Y-%m-%dT%H:%M:%SZ) +" + patch_closing_comment_append "$AUDIT_LINE" + exit 0 # proceed to Step 7 +fi + +# Silent skip: env var rollback +if [ "$IDD_DISTRIBUTION_SYNC_PROMPT" = "false" ]; then + AUDIT_LINE="### Distribution Sync + +- **Detected**: $DISTRIBUTION_TYPE +- **Choice**: prompt skipped (IDD_DISTRIBUTION_SYNC_PROMPT=false, per IC_R011-style rollback) +- **At**: $(date -u +%Y-%m-%dT%H:%M:%SZ) +" + patch_closing_comment_append "$AUDIT_LINE" + exit 0 +fi +``` + +If detection hit → enter the AskUserQuestion deliberation moment described below. + +#### AskUserQuestion 3-option (prose — NOT a bash function call) + +> **Why prose**: AskUserQuestion is a Claude Code agent-level tool, not a bash function. Embedding `AskUserQuestion(...)` inside fenced bash was a category error caught in #47 P1 finding 2 (idd-all-chain Step 0.4). Same pattern applies here — agent reads bash detection, branches at agent level on `DISTRIBUTION_TYPE != "n/a"`, then handles deliberation as prose. + +When `DISTRIBUTION_TYPE` is plugin/MCP/CLI/mixed, the agent invokes **AskUserQuestion** with this question structure (per IC_R011 canonical 3-option pattern): + +> "Issue #${NUMBER} closed. Detected distribution type: **${DISTRIBUTION_TYPE}**. Sync to user-facing channel?" +> +> Options (default = first): +> - **`chain to ${SKILL_NAME} now`** — invoke `${SKILL_NAME}` (resolved per D3 superset table below);outcome recorded in audit trail +> - **`skip — I'll sync manually later`** — record `### Distribution Sync Pending` audit + provide manual command;close completes +> - **`not applicable — this issue doesn't ship`** — record `### Distribution Sync — Not Applicable`;e.g. test infra fix, internal refactor, docs-only change that doesn't reach user + +#### D3 Skill resolution table + +| `DISTRIBUTION_TYPE` | `SKILL_NAME` to invoke | +|---------------------|----------------------| +| `plugin` | `/plugin-tools:plugin-update ` | +| `mcp` | `/mcp-tools:mcp-deploy` | +| `cli` | `/cli-tools:cli-deploy` | +| `plugin+mcp` / `plugin+cli` | `/plugin-tools:plugin-update ` (D3 superset rule — plugin-update v1.11+ Phase 1.5 cascades to binary deploy) | + +**Plugin name resolution**: `` is the `name` field of the matched `marketplace.json` `plugins[]` entry (algorithm in `references/distribution-detection.md`). + +**v1 caller-flag note**: Step 6.5 v1 invokes target skill **without** `--source-issue` flag (caller skills in `psychquant-claude-plugins` repo don't yet support it — sister issue tracks). Graceful: target skills ignore unknown flags. Audit trail still complete via closing comment patch. + +#### Audit trail PATCH (any choice) + +Regardless of user's choice, PATCH the closing comment (captured in Step 4 as `$CLOSING_COMMENT_ID`) to append `### Distribution Sync` section: + +```bash +# Helper: append a block to closing comment via gh api PATCH +patch_closing_comment_append() { + local append_block="$1" + local existing=$(gh api "/repos/${GITHUB_REPO}/issues/comments/${CLOSING_COMMENT_ID}" -q .body) + jq -n --arg b "${existing}${append_block}" '{body: $b}' > /tmp/distribution_sync_patch.json + gh api -X PATCH "/repos/${GITHUB_REPO}/issues/comments/${CLOSING_COMMENT_ID}" \ + --input /tmp/distribution_sync_patch.json -q '.html_url' +} + +# Build outcome line per choice +case "$USER_CHOICE" in + "chain to "*) + # Invoke target skill via Skill(...) tool, capture outcome summary + # Skill(skill="plugin-tools:plugin-update", args="") + OUTCOME="invoked \`${SKILL_NAME}\`, see chained skill output above" + ;; + "skip"*) + OUTCOME="pending — run \`${SKILL_NAME}\` when ready" + ;; + "not applicable"*) + OUTCOME="this fix doesn't reach user-facing distribution surface" + ;; +esac + +AUDIT_BLOCK=" + +### Distribution Sync + +- **Detected**: ${DISTRIBUTION_TYPE} +- **Choice**: ${USER_CHOICE} +- **Outcome**: ${OUTCOME} +- **At**: $(date -u +%Y-%m-%dT%H:%M:%SZ) +" + +patch_closing_comment_append "$AUDIT_BLOCK" +``` + +#### Failure-mode summary + +| Scenario | Behavior | +|----------|----------| +| Non-distribution repo | Silent skip + 1-line audit `(detection: not a distribution repo)` | +| `IDD_DISTRIBUTION_SYNC_PROMPT=false` | Silent skip + 1-line audit citing env var | +| Detection hit + user picks `chain` | Skill invoked + outcome appended to audit trail | +| Detection hit + user picks `skip` | Manual command recorded for later use | +| Detection hit + user picks `not applicable` | Reason recorded;close completes cleanly | +| `gh api PATCH` fails (network / auth) | Print warning;close still proceeds (audit trail loss is non-blocking) | +| Plugin name resolution fails (marketplace.json malformed) | Treat as `n/a` + audit line `(detection error: marketplace.json parse failed)` | + +> **Future hook for `idd-implement` Step 5.x early-surface**: same `infer_distribution_type` helper could power an earlier prompt (at PR-open time rather than close time). Out of scope for #45 v1 — separate follow-up if value confirmed. + ### Step 7: 批次 close 特殊規則 若這次 `/idd-close` 是批次的其中一輪（例如 archive 之後同時 close #1 #2 #3），Step 5 和 Step 6 要對**每個 issue 各跑一次**。不要把多個 issue 的回報合併。TaskCreate 清單裡為每個 issue id 各建一份 `auto_update_body_N`。 From 023748cbc4cfa8533eace932ea2b473ef4f3aa44 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 10 May 2026 18:21:03 +0800 Subject: [PATCH 3/6] fix(idd-close): Step 4 capture closing comment ID for Step 6.5 PATCH (Refs #45) In-scope dependency caught during Step 5.7 sister bug sweep: Step 6.5 PATCH references $CLOSING_COMMENT_ID expected to be set in Step 4, but Step 4 was a fire-and-forget gh issue comment without URL capture. Without this fix, Step 6.5 audit trail PATCH would fail with empty ID. 3-line addition: capture URL via tail -1 + extract numeric ID via sed regex. No behavior change for non-distribution repos (Step 6.5 silent-skips before PATCH path is reached). --- plugins/issue-driven-dev/skills/idd-close/SKILL.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/issue-driven-dev/skills/idd-close/SKILL.md b/plugins/issue-driven-dev/skills/idd-close/SKILL.md index fb0311b..8daacb2 100644 --- a/plugins/issue-driven-dev/skills/idd-close/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-close/SKILL.md @@ -404,7 +404,10 @@ For each trigger-phrase match: ### Step 4: 發佈並關閉 ```bash -gh issue comment $NUMBER --repo $GITHUB_REPO --body "$CLOSING_COMMENT" +# Capture the comment URL → ID for downstream PATCH (Step 6.5 Distribution Sync audit trail). +# Tail -1 grabs the URL line `gh` prints to stdout; sed extracts the numeric ID. +CLOSING_COMMENT_URL=$(gh issue comment $NUMBER --repo $GITHUB_REPO --body "$CLOSING_COMMENT" 2>&1 | tail -1) +CLOSING_COMMENT_ID=$(echo "$CLOSING_COMMENT_URL" | sed -E 's/.*#issuecomment-([0-9]+)/\1/') gh issue close $NUMBER --repo $GITHUB_REPO ``` From 133354ce450e908f4e7e227e86557045835feb66 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 10 May 2026 18:50:18 +0800 Subject: [PATCH 4/6] fix(idd-close): round-2 P1 blockers from /idd-verify --pr 67 (Refs #45) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 1 verify (3-source convergence: Codex + regression + devil's advocate) flagged 5 P1 blocking findings. All 5 fixed in this round: F1 — marketplace.json schema mismatch (P0/P1): Real schema is "source": "./plugins/X" string, not {"git": "..."} object. Updated is_plugin_marketplace_member to: - parse plugins[].source as string (with select(type=="string") guard) - resolve relative to manifest dir + canonicalize via pwd -P - match if plugin_dir == repo_root OR plugin_dir is inside repo_root tree Empirically validated 5/5 cases including dogfood (this very repo → plugin). F2 — wrapper regex misses real-world wrappers (P0/P1): Real wrappers (che-apple-mail-mcp, agent-cacher) construct GitHub URLs in variables (API_URL=, asset_url=) and call curl/gh api on a separate line. Old regex was line-anchored. New pattern matches anywhere in bin/*.sh: gh release download | gh api .* /releases | api.github.com/.*/releases | github.com/.*/releases/(download|tags|latest) Validated against 13 PsychQuant MCP wrappers + agent-cacher CLI wrapper. F3 — helpers undefined when called (P1): Step 6.5 SKILL.md called infer_distribution_type and patch_closing_comment_append but defs lived only in references/distribution-detection.md. Inlined helpers inside Step 6.5 detection block, BEFORE first call. Removed duplicate def in audit trail section (now references the inlined version). F4 — Step 4 closing comment ID capture brittle (P1): Old: `gh issue comment ... 2>&1 | tail -1` mixed stderr into stdout; sed without -n returned input unchanged on no-match → silent corruption. New: capture stdout-only with || abort on gh failure; use sed -nE '...p' to print only on match; explicit empty-check with abort. F5 — D3 superset rule unverified (P1): Plan/contract claimed "chain plugin-update only" relying on Phase 1.5 cascade per common-plugins.md (filed #66 to audit empirically). Until #66 confirms, v1 default is "chain both with explicit ordering" (binary-deploy first, then plugin-update). Idempotent regardless of cascade behavior. v2 superset rule pending #66. Bonus fixes: - F6 (P3): replaced `exit 0 # proceed to Step 7` literal bash exit (which would terminate close skill) with agent-level branching prose comment. - == /Users/che edge case fixed via do-while-style loop. - Symlink resolution via `pwd -P` for /Users vs /private/var on macOS. --- .../references/distribution-detection.md | 105 ++++++++--- .../skills/idd-close/SKILL.md | 167 ++++++++++++++---- 2 files changed, 215 insertions(+), 57 deletions(-) diff --git a/plugins/issue-driven-dev/references/distribution-detection.md b/plugins/issue-driven-dev/references/distribution-detection.md index 992f662..3301999 100644 --- a/plugins/issue-driven-dev/references/distribution-detection.md +++ b/plugins/issue-driven-dev/references/distribution-detection.md @@ -31,33 +31,57 @@ Walk-up parents from `$REPO_ROOT` looking for `.claude-plugin/marketplace.json`, ```bash is_plugin_marketplace_member() { local repo_root="$1" - local github_repo="$2" # owner/repo form - local current="$repo_root" - - while [ "$current" != "$HOME" ] && [ "$current" != "/" ]; do + local github_repo="$2" # owner/repo form (unused in v1 — kept for v2 git-remote variant) + local resolved_root + resolved_root=$(cd "$repo_root" 2>/dev/null && pwd -P) || resolved_root="$repo_root" + local current="$resolved_root" + + # Walk up to (and including) $HOME. The do-while-style first-pass check below + # ensures `$REPO_ROOT == $HOME` doesn't fall through unscanned. + while :; do local manifest="$current/.claude-plugin/marketplace.json" if [ -f "$manifest" ]; then - # Check if any plugins[].source.git matches our repo - # Normalize trailing .git for comparison - local match=$(jq -r --arg repo "$github_repo" \ - '.plugins[]?.source.git // empty - | sub(".git$"; "") - | select(endswith($repo))' \ - "$manifest" 2>/dev/null | head -1) + # Marketplace plugin schema: `"source": "./plugins/"` (string, relative + # to manifest dir). Resolve each source path and compare to repo_root — + # match means this repo IS one of the plugins listed by the marketplace. + local manifest_dir + manifest_dir=$(cd "$(dirname "$manifest")/.." 2>/dev/null && pwd -P) || \ + manifest_dir=$(dirname "$(dirname "$manifest")") + local match + match=$(jq -r '.plugins[]?.source // empty | select(type == "string")' \ + "$manifest" 2>/dev/null \ + | while IFS= read -r src; do + [ -z "$src" ] && continue + local plugin_dir + plugin_dir=$(cd "$manifest_dir/$src" 2>/dev/null && pwd -P) || continue + # Match if plugin_dir IS repo_root (repo == one plugin) + # OR plugin_dir is INSIDE repo_root (repo hosts plugins under plugins/, common monorepo layout). + # Trailing slash check prevents prefix collisions (e.g. repo=/foo, plugin_dir=/foobar). + case "$plugin_dir/" in + "$resolved_root/"|"$resolved_root/"*) + echo "match" + break + ;; + esac + done | head -1) if [ -n "$match" ]; then echo "true" return 0 fi fi + [ "$current" = "$HOME" ] && break + [ "$current" = "/" ] && break current=$(dirname "$current") done echo "false" } ``` -**Stop conditions**: walking past `$HOME` is a strong signal we've left the user's project tree;`/` is a safety bound. Marketplace repos are by convention under `$HOME/Developer/` or similar. +**Stop conditions**: walks up through `$HOME` itself (so repos cloned directly to `$HOME` are scanned), then breaks. `/` is a safety bound. Path resolution via `pwd -P` normalizes symlinks (handles macOS `/Users` ↔ `/private/var/...` cases). + +**Match logic**: marketplace.json plugins use `"source": "./plugins/"` (string, relative to manifest dir). For each plugin entry, resolve `/` to absolute path and compare to canonicalized `$REPO_ROOT`. A match means this repo IS one of the plugins published by that marketplace — could be the marketplace's own repo (self-publishing) or a submodule path. -**Match logic**: `jq` extracts `plugins[].source.git` URLs (e.g. `https://github.com/PsychQuant/issue-driven-development.git`), strips trailing `.git`, then checks if any URL ends with the `owner/repo` form. This is robust to https vs ssh URLs and trailing `.git` variations. +**Schema note**: object form `{"source": {"git": "..."}}` is not currently used in any inspected real marketplace.json (37/37 use string form). If future marketplace.json adopts object form, jq filter would need a parallel branch — but the `select(type == "string")` guard makes the current filter safe (skips non-string sources rather than erroring). ### `has_binary_wrapper(repo_root) -> bool` @@ -70,16 +94,43 @@ has_binary_wrapper() { [ -d "$bin_dir" ] || { echo "false"; return; } - local match=$(ls "$bin_dir"/*.sh 2>/dev/null \ - | xargs grep -l -E 'gh release download|curl.*github\.com.*releases' 2>/dev/null \ - | head -1) + # Real-world wrappers (e.g. `che-apple-mail-mcp-wrapper.sh`, `agent-cacher`) + # construct GitHub URLs in variables (`API_URL=https://api.github.com/...`, + # `asset_url="https://github.com/.../releases/download/..."`) then call + # curl on a separate line. Line-anchored regex misses these. The pattern + # below matches any line containing GitHub release URL patterns OR explicit + # `gh release download` / `gh api .../releases` invocations. + # Match either explicit gh CLI invocations or GitHub URL patterns. Use .* between + # github.com and `releases` to accept variable-substituted single-segment paths + # (e.g. `https://github.com/${GITHUB_REPO}/releases/...` where the variable holds + # the full `owner/repo`); grep is line-based so .* won't span lines. + local pattern='gh release download|gh api[[:space:]]+.*/releases|api\.github\.com/.*/releases|github\.com/.*/releases/(download|tags|latest)' + + # Glob may not match — guard with a 2>/dev/null + null-check + local match="" + shopt -s nullglob 2>/dev/null + for f in "$bin_dir"/*.sh; do + [ -f "$f" ] || continue + if grep -qE "$pattern" "$f" 2>/dev/null; then + match="$f" + break + fi + done + shopt -u nullglob 2>/dev/null + [ -n "$match" ] && echo "true" || echo "false" } ``` **Why `bin/*.sh` only**: build/dev scripts typically live under `scripts/` or `Sources/`;user-facing distribution wrappers conventionally live under `bin/`. False-positive on a non-distribution `bin/*.sh` is covered by AskUserQuestion `not applicable` opt-out (see Step 6.5 prose). -**Detection keywords**: limited to GitHub-native distribution per #45 issue body scope. Future extension covered below. +**Detection keywords (broadened from v1)**: matches: +- `gh release download` — explicit GitHub CLI download +- `gh api[[:space:]]+.*/releases` — GitHub API via gh CLI +- `api\.github\.com/repos/.../releases` — direct REST API URL (real-world pattern: `che-apple-mail-mcp-wrapper.sh`) +- `github\.com/.../releases/(download|tags|latest)` — public release URLs (real-world pattern: `cacher-mcp-wrapper.sh` `asset_url=`) + +Empirically validated: matches all 13 PsychQuant MCP wrappers + `idd-route` CLI wrapper. Future patterns (wget, npm) covered by Extension protocol below. ### `infer_distribution_type(repo_root, github_repo) -> string` @@ -117,23 +168,25 @@ infer_distribution_type() { } ``` -## D3 — Mixed-type superset rule +## D3 — Mixed-type handling (v1: explicit ordering, fallback-safe) + +When `infer_distribution_type` returns `plugin+mcp` or `plugin+cli`, **chain BOTH skills with explicit ordering**: binary-deploy first (`mcp-deploy` or `cli-deploy`), then `plugin-update` to bump plugin shell pointing at the new binary version. -When `infer_distribution_type` returns `plugin+mcp` or `plugin+cli`, **chain `plugin-update` only**. +**Rationale (v1)**: ordering binary-first is correctness-safe regardless of whether `plugin-update` cascades. If cascade exists (per `common-plugins.md` claim), calling binary-deploy first means `plugin-update`'s Phase 1.5 detection sees the already-bumped binary and skips its own cascade prompt (idempotent). If cascade doesn't exist or has changed, calling both ensures both shells stay in sync — exact anti-pattern #45 was designed to prevent. -**Rationale**: per `che-claude-config/rules/common-plugins.md`, `plugin-tools:plugin-update` v1.11+ is **dependency-aware orchestrator** — its Phase 1.5 detects `.mcp.json` / wrapper / session-start hook dependencies and AskUserQuestion-prompts the cascade ("順便更新 binary?" → invokes `mcp-deploy` / `cli-deploy`). Calling `plugin-update` is the superset action;calling both separately would either duplicate work or skip the plugin shell sync. +**Why not v2 superset rule yet**: the original Plan tier proposed "chain `plugin-update` only" relying on Phase 1.5 cascade per `che-claude-config/rules/common-plugins.md`. PsychQuant/issue-driven-development#66 audit was filed during /idd-plan to verify this claim empirically before relying on it. Until #66 confirms cascade behavior matches doc claim, **v1 default is "chain both" for safety**. -**Caveat**: this claim is **unverified empirically** (filed as PsychQuant/issue-driven-development#66 audit). If audit reveals stale claim, this contract should change to "chain both with explicit ordering" — `mcp-deploy` first to bump binary, then `plugin-update` to bump plugin shell pointing at new binary version. +**Future v2 (post-#66 audit)**: if #66 confirms `plugin-update` v1.11+ Phase 1.5 reliably cascades to `mcp-deploy` / `cli-deploy`, this contract may change to "chain `plugin-update` only" superset rule. v2 contract would document explicit cascade dependency on `plugin-update` v1.11+ and include version check. -## Skill resolution table +## Skill resolution table (v1) -| `infer_distribution_type` returns | Sync skill to chain | -|----------------------------------|--------------------| +| `infer_distribution_type` returns | Sync skills to chain (in order) | +|----------------------------------|---------------------------------| | `plugin` | `/plugin-tools:plugin-update ` | | `mcp` | `/mcp-tools:mcp-deploy` | | `cli` | `/cli-tools:cli-deploy` | -| `plugin+mcp` | `/plugin-tools:plugin-update ` (D3) | -| `plugin+cli` | `/plugin-tools:plugin-update ` (D3) | +| `plugin+mcp` | `/mcp-tools:mcp-deploy` → then `/plugin-tools:plugin-update ` | +| `plugin+cli` | `/cli-tools:cli-deploy` → then `/plugin-tools:plugin-update ` | | `n/a` | — silent skip, no chain | **Plugin name resolution** (when `` is needed): parse the matched `marketplace.json` entry — its `name` field IS the plugin name to pass to `plugin-update`. diff --git a/plugins/issue-driven-dev/skills/idd-close/SKILL.md b/plugins/issue-driven-dev/skills/idd-close/SKILL.md index 8daacb2..05df894 100644 --- a/plugins/issue-driven-dev/skills/idd-close/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-close/SKILL.md @@ -405,9 +405,23 @@ For each trigger-phrase match: ```bash # Capture the comment URL → ID for downstream PATCH (Step 6.5 Distribution Sync audit trail). -# Tail -1 grabs the URL line `gh` prints to stdout; sed extracts the numeric ID. -CLOSING_COMMENT_URL=$(gh issue comment $NUMBER --repo $GITHUB_REPO --body "$CLOSING_COMMENT" 2>&1 | tail -1) -CLOSING_COMMENT_ID=$(echo "$CLOSING_COMMENT_URL" | sed -E 's/.*#issuecomment-([0-9]+)/\1/') +# IMPORTANT: don't use `2>&1 | tail -1` — that mixes stderr into stdout, and gh's +# deprecation/warning messages can interleave with the URL, leading to a garbage ID. +# `gh issue comment` prints ONLY the comment URL to stdout on success;errors go to stderr. +# `set -e` (or explicit `|| abort`) guarantees we don't proceed with empty URL on failure. +CLOSING_COMMENT_URL=$(gh issue comment $NUMBER --repo $GITHUB_REPO --body "$CLOSING_COMMENT") || { + echo "ERROR: gh issue comment failed for #$NUMBER" >&2 + exit 1 +} + +# `sed -n '...p'` only prints the line if the regex matched, so a malformed URL +# yields an empty CLOSING_COMMENT_ID rather than the unmodified input string. +CLOSING_COMMENT_ID=$(printf '%s' "$CLOSING_COMMENT_URL" | sed -nE 's|.*#issuecomment-([0-9]+).*|\1|p') +if [ -z "$CLOSING_COMMENT_ID" ]; then + echo "ERROR: failed to extract comment ID from URL: $CLOSING_COMMENT_URL" >&2 + exit 1 +fi + gh issue close $NUMBER --repo $GITHUB_REPO ``` @@ -482,40 +496,131 @@ Skill(skill="issue-driven-dev:idd-update", args="#NNN") **Placement rationale**: phase=closed locks audit at Step 6 (lifecycle gate complete);Step 6.5 is follow-up action not lifecycle gate. The closing comment's `### Distribution Sync` section heading clearly separates from main summary so audit trail timeline is unambiguous. -#### Detection (bash) +#### Detection (bash) — define helpers inline, then call -Per [`references/distribution-detection.md`](../../references/distribution-detection.md) canonical contract — implement helpers inline (or source the contract's algorithm): +Per [`references/distribution-detection.md`](../../references/distribution-detection.md) canonical contract. **Helpers MUST be defined before first call** — the silent-skip path below references them, so define-then-call ordering matters (bash treats undefined function calls as `command not found` exit 127). ```bash -# Detection delegated to contract (algorithm fully spec'd in distribution-detection.md) -DISTRIBUTION_TYPE=$(infer_distribution_type "$REPO_ROOT" "$GITHUB_REPO") +# ── Helpers (inlined from references/distribution-detection.md) ── + +# patch_closing_comment_append — appends a markdown block to the closing comment +# captured in Step 4 ($CLOSING_COMMENT_ID). All silent-skip and prompt-resolution +# paths in this step call this helper, so define it FIRST. +patch_closing_comment_append() { + local append_block="$1" + local existing + existing=$(gh api "/repos/${GITHUB_REPO}/issues/comments/${CLOSING_COMMENT_ID}" -q .body 2>/dev/null) || { + echo "WARN: gh api fetch failed for comment $CLOSING_COMMENT_ID — audit trail PATCH skipped" >&2 + return 0 + } + jq -n --arg b "${existing}${append_block}" '{body: $b}' > /tmp/distribution_sync_patch.json + gh api -X PATCH "/repos/${GITHUB_REPO}/issues/comments/${CLOSING_COMMENT_ID}" \ + --input /tmp/distribution_sync_patch.json -q '.html_url' >/dev/null || \ + echo "WARN: gh api PATCH failed — audit trail incomplete" >&2 +} + +# is_plugin_marketplace_member — walk-up scan for marketplace.json listing this repo +is_plugin_marketplace_member() { + local repo_root="$1" + local resolved_root + resolved_root=$(cd "$repo_root" 2>/dev/null && pwd -P) || resolved_root="$repo_root" + local current="$resolved_root" + while :; do + local manifest="$current/.claude-plugin/marketplace.json" + if [ -f "$manifest" ]; then + local manifest_dir + manifest_dir=$(cd "$(dirname "$manifest")/.." 2>/dev/null && pwd -P) || \ + manifest_dir=$(dirname "$(dirname "$manifest")") + local match + match=$(jq -r '.plugins[]?.source // empty | select(type == "string")' \ + "$manifest" 2>/dev/null \ + | while IFS= read -r src; do + [ -z "$src" ] && continue + local plugin_dir + plugin_dir=$(cd "$manifest_dir/$src" 2>/dev/null && pwd -P) || continue + case "$plugin_dir/" in + "$resolved_root/"|"$resolved_root/"*) echo "match"; break ;; + esac + done | head -1) + [ -n "$match" ] && { echo "true"; return 0; } + fi + [ "$current" = "$HOME" ] && break + [ "$current" = "/" ] && break + current=$(dirname "$current") + done + echo "false" +} + +# has_binary_wrapper — scan bin/*.sh for GitHub release patterns (line-agnostic) +has_binary_wrapper() { + local repo_root="$1" + local bin_dir="$repo_root/bin" + [ -d "$bin_dir" ] || { echo "false"; return; } + local pattern='gh release download|gh api[[:space:]]+.*/releases|api\.github\.com/.*/releases|github\.com/.*/releases/(download|tags|latest)' + local match="" + shopt -s nullglob 2>/dev/null + for f in "$bin_dir"/*.sh; do + [ -f "$f" ] || continue + if grep -qE "$pattern" "$f" 2>/dev/null; then match="$f"; break; fi + done + shopt -u nullglob 2>/dev/null + [ -n "$match" ] && echo "true" || echo "false" +} + +# infer_distribution_type — orchestrator returning plugin/mcp/cli/plugin+mcp/plugin+cli/n/a +infer_distribution_type() { + local repo_root="$1" + local is_plugin=$(is_plugin_marketplace_member "$repo_root") + local has_wrapper=$(has_binary_wrapper "$repo_root") + local binary_kind="" + if [ "$has_wrapper" = "true" ]; then + local mcp_match=$(ls "$repo_root/bin"/*.sh 2>/dev/null \ + | grep -E '(-mcp-wrapper\.sh$|/[^/]*mcp[^/]*\.sh$)' | head -1) + [ -n "$mcp_match" ] && binary_kind="mcp" || binary_kind="cli" + fi + if [ "$is_plugin" = "true" ] && [ -n "$binary_kind" ]; then + echo "plugin+$binary_kind" + elif [ "$is_plugin" = "true" ]; then echo "plugin" + elif [ -n "$binary_kind" ]; then echo "$binary_kind" + else echo "n/a"; fi +} + +# ── Detection invocation ── +DISTRIBUTION_TYPE=$(infer_distribution_type "$REPO_ROOT") # Returns: plugin / mcp / cli / plugin+mcp / plugin+cli / n/a # Silent skip: non-distribution repo if [ "$DISTRIBUTION_TYPE" = "n/a" ]; then - AUDIT_LINE="### Distribution Sync + AUDIT_LINE=" + +### Distribution Sync - **Detected**: not a distribution repo (skipped) - **At**: $(date -u +%Y-%m-%dT%H:%M:%SZ) " patch_closing_comment_append "$AUDIT_LINE" - exit 0 # proceed to Step 7 + # Silent skip — proceed to Step 7 (do NOT use `exit 0` here; in markdown-skill + # context that would terminate the entire close flow including Step 7 batch + # close special rules. Instead, the agent reading this skill branches at + # agent level: `n/a` path is complete, advance to Step 7 in skill order.) fi -# Silent skip: env var rollback -if [ "$IDD_DISTRIBUTION_SYNC_PROMPT" = "false" ]; then - AUDIT_LINE="### Distribution Sync +# Silent skip: env var rollback (overrides prompt for distribution-detected repos) +if [ "$DISTRIBUTION_TYPE" != "n/a" ] && [ "$IDD_DISTRIBUTION_SYNC_PROMPT" = "false" ]; then + AUDIT_LINE=" + +### Distribution Sync - **Detected**: $DISTRIBUTION_TYPE - **Choice**: prompt skipped (IDD_DISTRIBUTION_SYNC_PROMPT=false, per IC_R011-style rollback) - **At**: $(date -u +%Y-%m-%dT%H:%M:%SZ) " patch_closing_comment_append "$AUDIT_LINE" - exit 0 + # Same agent-level advance: skip prompt, proceed to Step 7. fi ``` -If detection hit → enter the AskUserQuestion deliberation moment described below. +If detection returned a real type AND env var didn't suppress → enter the AskUserQuestion deliberation moment described below. #### AskUserQuestion 3-option (prose — NOT a bash function call) @@ -530,38 +635,38 @@ When `DISTRIBUTION_TYPE` is plugin/MCP/CLI/mixed, the agent invokes **AskUserQue > - **`skip — I'll sync manually later`** — record `### Distribution Sync Pending` audit + provide manual command;close completes > - **`not applicable — this issue doesn't ship`** — record `### Distribution Sync — Not Applicable`;e.g. test infra fix, internal refactor, docs-only change that doesn't reach user -#### D3 Skill resolution table +#### D3 Skill resolution table (v1: explicit ordering, fallback-safe) + +Per #45 Plan tier D3 + #66 audit dependency: until `plugin-update` Phase 1.5 cascade is empirically confirmed (see #66), v1 chains BOTH skills explicitly for mixed types — binary-deploy first, then plugin-update. -| `DISTRIBUTION_TYPE` | `SKILL_NAME` to invoke | -|---------------------|----------------------| +| `DISTRIBUTION_TYPE` | Sync skill(s) to invoke (in order) | +|---------------------|-----------------------------------| | `plugin` | `/plugin-tools:plugin-update ` | | `mcp` | `/mcp-tools:mcp-deploy` | | `cli` | `/cli-tools:cli-deploy` | -| `plugin+mcp` / `plugin+cli` | `/plugin-tools:plugin-update ` (D3 superset rule — plugin-update v1.11+ Phase 1.5 cascades to binary deploy) | +| `plugin+mcp` | `/mcp-tools:mcp-deploy` → then `/plugin-tools:plugin-update ` | +| `plugin+cli` | `/cli-tools:cli-deploy` → then `/plugin-tools:plugin-update ` | **Plugin name resolution**: `` is the `name` field of the matched `marketplace.json` `plugins[]` entry (algorithm in `references/distribution-detection.md`). +**Why ordering matters for mixed types**: bumping the binary first means `plugin-update`'s Phase 1.5 dependency check (if cascade exists) sees the already-bumped binary and skips its own cascade prompt — idempotent. If `plugin-update` doesn't cascade (per #66 audit risk), explicit ordering ensures both shells are still in sync. Either way safer than relying on superset behavior unverified. + **v1 caller-flag note**: Step 6.5 v1 invokes target skill **without** `--source-issue` flag (caller skills in `psychquant-claude-plugins` repo don't yet support it — sister issue tracks). Graceful: target skills ignore unknown flags. Audit trail still complete via closing comment patch. #### Audit trail PATCH (any choice) -Regardless of user's choice, PATCH the closing comment (captured in Step 4 as `$CLOSING_COMMENT_ID`) to append `### Distribution Sync` section: +Regardless of user's choice, PATCH the closing comment (captured in Step 4 as `$CLOSING_COMMENT_ID`) to append `### Distribution Sync` section. The `patch_closing_comment_append` helper was defined inline at the top of this Step 6.5 detection block (above) — reuse it here. ```bash -# Helper: append a block to closing comment via gh api PATCH -patch_closing_comment_append() { - local append_block="$1" - local existing=$(gh api "/repos/${GITHUB_REPO}/issues/comments/${CLOSING_COMMENT_ID}" -q .body) - jq -n --arg b "${existing}${append_block}" '{body: $b}' > /tmp/distribution_sync_patch.json - gh api -X PATCH "/repos/${GITHUB_REPO}/issues/comments/${CLOSING_COMMENT_ID}" \ - --input /tmp/distribution_sync_patch.json -q '.html_url' -} - -# Build outcome line per choice +# Build outcome line per choice. Match against the AskUserQuestion option labels +# emitted in the prose section above (substring match on the leading verb works +# because options have distinct first words: "chain", "skip", "not applicable"). case "$USER_CHOICE" in "chain to "*) - # Invoke target skill via Skill(...) tool, capture outcome summary - # Skill(skill="plugin-tools:plugin-update", args="") + # Invoke target skill via Skill(...) tool at agent level (NOT inside this + # bash block — the Skill tool is a Claude Code agent-level tool). Per D3 + # mixed-type ordering: for plugin+mcp/plugin+cli, invoke binary-deploy + # first, then plugin-update. Capture outcome summary from each invocation. OUTCOME="invoked \`${SKILL_NAME}\`, see chained skill output above" ;; "skip"*) From 3ba6ad1c5af7bedf3f5b8baeeeadc05ee0a8dedb Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 10 May 2026 19:34:20 +0800 Subject: [PATCH 5/6] =?UTF-8?q?fix(idd-close):=20round-3=20=E2=80=94=20REP?= =?UTF-8?q?O=5FROOT=20init=20+=20plugin=20name=20resolution=20+=20doc=20co?= =?UTF-8?q?nsistency=20(Refs=20#45)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 verify caught 2 NEW P1 introduced by round-2 + 2 PARTIAL doc inconsistencies: NEW-P1-1: REPO_ROOT undefined causing infinite loop / false positive on legacy repos - idd-close didn't define REPO_ROOT before Step 6.5 detection invocation - Empty string triggered: cd "" succeeds → resolved_root='', dirname '.' loops forever - Fix: Step 6.5 prelude resolves via 'git rev-parse --show-toplevel' (fallback pwd) - If unresolvable (not in git tree), audit-skip with explicit reason + advance to Step 7 NEW-P1-2: SKILL_NAME / plugin name not mechanically resolved → audit refs undefined var - Old code referenced $SKILL_NAME but never set it from helper output - is_plugin_marketplace_member returned only boolean — no name extraction - Fix: NEW resolve_plugin_name() helper returns matched plugin's .name field - Detection invocation now sets PLUGIN_NAME for plugin/plugin+* types - Composes $CHAIN_DESC table-driven from (DISTRIBUTION_TYPE, PLUGIN_NAME) before AskUserQuestion - If type=plugin* but PLUGIN_NAME unresolvable → demote to n/a (avoid malformed chain) - Replaces $SKILL_NAME with $CHAIN_DESC throughout audit/AskUserQuestion blocks PARTIAL-1: Top-level summary table still said "plugins[].source.git" (object schema) - Fix: row 17 now says "plugins[].source (string \"./plugins/\")" - row 18 'mcp' detection signal updated to enumerate broadened pattern - Mixed types now show explicit ordering chain (D3 v1) instead of plugin-update only PARTIAL-2: Failure-mode summary claimed marketplace.json malformed → audit error - jq 2>/dev/null swallowed parse errors → no error audit fired - Fix: clarify behavior is "demote to n/a + WARN to stderr";simpler audit schema Smoke validated round-3: - resolve_plugin_name correctly returns 'issue-driven-dev' for this repo, 'che-apple-mail-mcp' for plugin dir, empty for che-claude-config (not a plugin), first match for monorepo host - REPO_ROOT git rev-parse works in-tree;pwd fallback for non-git contexts --- .../references/distribution-detection.md | 52 ++++++++- .../skills/idd-close/SKILL.md | 100 ++++++++++++++++-- 2 files changed, 141 insertions(+), 11 deletions(-) diff --git a/plugins/issue-driven-dev/references/distribution-detection.md b/plugins/issue-driven-dev/references/distribution-detection.md index 3301999..e0c546e 100644 --- a/plugins/issue-driven-dev/references/distribution-detection.md +++ b/plugins/issue-driven-dev/references/distribution-detection.md @@ -14,10 +14,11 @@ This contract specifies how `/idd-close` Step 6.5 detects whether the closing re | Type | Detection signal | Sync skill | |------|-----------------|-----------| -| `plugin` | Repo is referenced as `plugins[].source.git` in some ancestor `.claude-plugin/marketplace.json` | `/plugin-tools:plugin-update ` | -| `mcp` | `$REPO_ROOT/bin/*.sh` contains `gh release download` or `curl.*github.com.*releases` AND wrapper script name matches `*-mcp-wrapper.sh` (or contains `mcp` substring) | `/mcp-tools:mcp-deploy` | +| `plugin` | Repo or any subdir is listed as `plugins[].source` (string `"./plugins/"`) in some ancestor `.claude-plugin/marketplace.json` | `/plugin-tools:plugin-update ` | +| `mcp` | `$REPO_ROOT/bin/*.sh` contains GitHub release URL pattern (`gh release download` / `gh api .../releases` / `api.github.com/.../releases` / `github.com/.../releases/(download\|tags\|latest)` — line-agnostic, matches variable-substituted URLs) AND wrapper script name matches `*-mcp-wrapper.sh` (or contains `mcp` substring) | `/mcp-tools:mcp-deploy` | | `cli` | Same wrapper detection as MCP, but wrapper script name does NOT match MCP heuristic | `/cli-tools:cli-deploy` | -| `plugin+mcp` / `plugin+cli` | Both signals trigger | `/plugin-tools:plugin-update ` (per D3 superset rule, see below) | +| `plugin+mcp` | Both signals trigger | `/mcp-tools:mcp-deploy` → then `/plugin-tools:plugin-update ` (D3 v1: explicit ordering, see below) | +| `plugin+cli` | Both signals trigger | `/cli-tools:cli-deploy` → then `/plugin-tools:plugin-update ` (D3 v1: explicit ordering, see below) | | `n/a` | None of the above | — silent skip | ## Detection helpers @@ -132,6 +133,51 @@ has_binary_wrapper() { Empirically validated: matches all 13 PsychQuant MCP wrappers + `idd-route` CLI wrapper. Future patterns (wget, npm) covered by Extension protocol below. +### `resolve_plugin_name(repo_root) -> string` + +Returns the **plugin `name` field** from the marketplace.json entry whose `source` resolves to `repo_root` (or a subdir of it). Required when chain target is `plugin-update ` — Step 6.5 calls this AFTER `is_plugin_marketplace_member` returns true. Empty output if no match (caller must check). + +```bash +resolve_plugin_name() { + local repo_root="$1" + local resolved_root + resolved_root=$(cd "$repo_root" 2>/dev/null && pwd -P) || resolved_root="$repo_root" + local current="$resolved_root" + while :; do + local manifest="$current/.claude-plugin/marketplace.json" + if [ -f "$manifest" ]; then + local manifest_dir + manifest_dir=$(cd "$(dirname "$manifest")/.." 2>/dev/null && pwd -P) || \ + manifest_dir=$(dirname "$(dirname "$manifest")") + # Iterate plugin entries with both name and source. For each, resolve source + # path and check if matches repo_root prefix; emit name on match. + local name_match + name_match=$(jq -r '.plugins[]? | select(.source | type == "string") | "\(.name)\t\(.source)"' \ + "$manifest" 2>/dev/null \ + | while IFS=$'\t' read -r pname psrc; do + [ -z "$pname" ] && continue + local plugin_dir + plugin_dir=$(cd "$manifest_dir/$psrc" 2>/dev/null && pwd -P) || continue + case "$plugin_dir/" in + "$resolved_root/"|"$resolved_root/"*) + echo "$pname" + break + ;; + esac + done | head -1) + [ -n "$name_match" ] && { echo "$name_match"; return 0; } + fi + [ "$current" = "$HOME" ] && break + [ "$current" = "/" ] && break + current=$(dirname "$current") + done + # No match — return empty (caller checks via [ -z "$NAME" ]) + echo "" +} +``` + +**Monorepo host case**: when `repo_root` is the marketplace host itself (e.g. `psychquant-claude-plugins` containing 37 plugins), `resolve_plugin_name` returns the FIRST plugin whose source path starts within `repo_root`. This is ambiguous for monorepo hosts — caller (Step 6.5) should handle this case explicitly, e.g. by prompting user "which plugin?" via AskUserQuestion. v1 simplification: assume single-plugin per repo (true for most non-host cases); document monorepo handling as a v2 enhancement. + ### `infer_distribution_type(repo_root, github_repo) -> string` Orchestrator returning one of: `plugin` / `mcp` / `cli` / `plugin+mcp` / `plugin+cli` / `n/a`. diff --git a/plugins/issue-driven-dev/skills/idd-close/SKILL.md b/plugins/issue-driven-dev/skills/idd-close/SKILL.md index 05df894..700b690 100644 --- a/plugins/issue-driven-dev/skills/idd-close/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-close/SKILL.md @@ -501,6 +501,25 @@ Skill(skill="issue-driven-dev:idd-update", args="#NNN") Per [`references/distribution-detection.md`](../../references/distribution-detection.md) canonical contract. **Helpers MUST be defined before first call** — the silent-skip path below references them, so define-then-call ordering matters (bash treats undefined function calls as `command not found` exit 127). ```bash +# ── Step 6.5 prelude: resolve REPO_ROOT (must be non-empty before helpers fire) ── +# Earlier steps don't define REPO_ROOT. Detection helpers depend on absolute path +# resolution + walk-up — empty input causes infinite loops in `dirname` and false +# positives via `cd "" && pwd`. Resolve from git first;fallback to PWD. +REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || pwd) +if [ -z "$REPO_ROOT" ] || [ ! -d "$REPO_ROOT" ]; then + echo "WARN: cannot resolve REPO_ROOT — Step 6.5 silent-skip" >&2 + AUDIT_LINE=" + +### Distribution Sync + +- **Detected**: skipped (REPO_ROOT not resolvable — likely not in git work tree) +- **At**: $(date -u +%Y-%m-%dT%H:%M:%SZ) +" + # Note: patch_closing_comment_append requires CLOSING_COMMENT_ID to be set; + # if it's also empty, the helper warns and returns without aborting close. + # Agent: skip remaining Step 6.5 logic, advance to Step 7 in skill order. +fi + # ── Helpers (inlined from references/distribution-detection.md) ── # patch_closing_comment_append — appends a markdown block to the closing comment @@ -519,6 +538,40 @@ patch_closing_comment_append() { echo "WARN: gh api PATCH failed — audit trail incomplete" >&2 } +# resolve_plugin_name — emit matched plugin's `name` field from marketplace entry +# (called AFTER is_plugin_marketplace_member returned true; required for SKILL_NAME +# composition `/plugin-tools:plugin-update `) +resolve_plugin_name() { + local repo_root="$1" + local resolved_root + resolved_root=$(cd "$repo_root" 2>/dev/null && pwd -P) || resolved_root="$repo_root" + local current="$resolved_root" + while :; do + local manifest="$current/.claude-plugin/marketplace.json" + if [ -f "$manifest" ]; then + local manifest_dir + manifest_dir=$(cd "$(dirname "$manifest")/.." 2>/dev/null && pwd -P) || \ + manifest_dir=$(dirname "$(dirname "$manifest")") + local name_match + name_match=$(jq -r '.plugins[]? | select(.source | type == "string") | "\(.name)\t\(.source)"' \ + "$manifest" 2>/dev/null \ + | while IFS=$'\t' read -r pname psrc; do + [ -z "$pname" ] && continue + local plugin_dir + plugin_dir=$(cd "$manifest_dir/$psrc" 2>/dev/null && pwd -P) || continue + case "$plugin_dir/" in + "$resolved_root/"|"$resolved_root/"*) echo "$pname"; break ;; + esac + done | head -1) + [ -n "$name_match" ] && { echo "$name_match"; return 0; } + fi + [ "$current" = "$HOME" ] && break + [ "$current" = "/" ] && break + current=$(dirname "$current") + done + echo "" +} + # is_plugin_marketplace_member — walk-up scan for marketplace.json listing this repo is_plugin_marketplace_member() { local repo_root="$1" @@ -589,6 +642,22 @@ infer_distribution_type() { DISTRIBUTION_TYPE=$(infer_distribution_type "$REPO_ROOT") # Returns: plugin / mcp / cli / plugin+mcp / plugin+cli / n/a +# Resolve plugin name when applicable (required for plugin-update chain). +# Empty string for pure mcp/cli/n/a — caller branches handle empty case. +PLUGIN_NAME="" +case "$DISTRIBUTION_TYPE" in + plugin|plugin+*) + PLUGIN_NAME=$(resolve_plugin_name "$REPO_ROOT") + if [ -z "$PLUGIN_NAME" ]; then + # Detection said plugin but name resolution failed — schema ambiguity. + # Demote to n/a (with audit) so we don't fire AskUserQuestion with + # malformed chain command. + echo "WARN: distribution type=$DISTRIBUTION_TYPE but plugin name unresolvable — demoting to n/a" >&2 + DISTRIBUTION_TYPE="n/a" + fi + ;; +esac + # Silent skip: non-distribution repo if [ "$DISTRIBUTION_TYPE" = "n/a" ]; then AUDIT_LINE=" @@ -626,12 +695,26 @@ If detection returned a real type AND env var didn't suppress → enter the AskU > **Why prose**: AskUserQuestion is a Claude Code agent-level tool, not a bash function. Embedding `AskUserQuestion(...)` inside fenced bash was a category error caught in #47 P1 finding 2 (idd-all-chain Step 0.4). Same pattern applies here — agent reads bash detection, branches at agent level on `DISTRIBUTION_TYPE != "n/a"`, then handles deliberation as prose. -When `DISTRIBUTION_TYPE` is plugin/MCP/CLI/mixed, the agent invokes **AskUserQuestion** with this question structure (per IC_R011 canonical 3-option pattern): +When `DISTRIBUTION_TYPE` is plugin/MCP/CLI/mixed, the agent **first** composes the chain command from the resolution table below using `$DISTRIBUTION_TYPE` and `$PLUGIN_NAME` (set in Detection invocation), then invokes **AskUserQuestion** per IC_R011 canonical 3-option pattern: + +```bash +# Compose chain command(s) — referenced as $CHAIN_DESC in AskUserQuestion below. +# For mixed types, this is two skills with explicit ordering (D3 v1). +case "$DISTRIBUTION_TYPE" in + plugin) CHAIN_DESC="/plugin-tools:plugin-update ${PLUGIN_NAME}" ;; + mcp) CHAIN_DESC="/mcp-tools:mcp-deploy" ;; + cli) CHAIN_DESC="/cli-tools:cli-deploy" ;; + plugin+mcp) CHAIN_DESC="/mcp-tools:mcp-deploy → /plugin-tools:plugin-update ${PLUGIN_NAME}" ;; + plugin+cli) CHAIN_DESC="/cli-tools:cli-deploy → /plugin-tools:plugin-update ${PLUGIN_NAME}" ;; +esac +``` + +Then AskUserQuestion: > "Issue #${NUMBER} closed. Detected distribution type: **${DISTRIBUTION_TYPE}**. Sync to user-facing channel?" > > Options (default = first): -> - **`chain to ${SKILL_NAME} now`** — invoke `${SKILL_NAME}` (resolved per D3 superset table below);outcome recorded in audit trail +> - **`chain to ${CHAIN_DESC} now`** — invoke the resolved skill(s) at agent level (Skill tool, NOT bash);outcome recorded in audit trail > - **`skip — I'll sync manually later`** — record `### Distribution Sync Pending` audit + provide manual command;close completes > - **`not applicable — this issue doesn't ship`** — record `### Distribution Sync — Not Applicable`;e.g. test infra fix, internal refactor, docs-only change that doesn't reach user @@ -663,14 +746,14 @@ Regardless of user's choice, PATCH the closing comment (captured in Step 4 as `$ # because options have distinct first words: "chain", "skip", "not applicable"). case "$USER_CHOICE" in "chain to "*) - # Invoke target skill via Skill(...) tool at agent level (NOT inside this + # Invoke target skill(s) via Skill(...) tool at agent level (NOT inside this # bash block — the Skill tool is a Claude Code agent-level tool). Per D3 # mixed-type ordering: for plugin+mcp/plugin+cli, invoke binary-deploy # first, then plugin-update. Capture outcome summary from each invocation. - OUTCOME="invoked \`${SKILL_NAME}\`, see chained skill output above" + OUTCOME="invoked \`${CHAIN_DESC}\`, see chained skill output above" ;; "skip"*) - OUTCOME="pending — run \`${SKILL_NAME}\` when ready" + OUTCOME="pending — run \`${CHAIN_DESC}\` when ready" ;; "not applicable"*) OUTCOME="this fix doesn't reach user-facing distribution surface" @@ -694,13 +777,14 @@ patch_closing_comment_append "$AUDIT_BLOCK" | Scenario | Behavior | |----------|----------| -| Non-distribution repo | Silent skip + 1-line audit `(detection: not a distribution repo)` | +| `REPO_ROOT` cannot be resolved (e.g. not in git work tree) | Silent skip + 1-line audit `(REPO_ROOT not resolvable)`;close still completes | +| Non-distribution repo (detection returns `n/a`) | Silent skip + 1-line audit `(detection: not a distribution repo)` | | `IDD_DISTRIBUTION_SYNC_PROMPT=false` | Silent skip + 1-line audit citing env var | | Detection hit + user picks `chain` | Skill invoked + outcome appended to audit trail | | Detection hit + user picks `skip` | Manual command recorded for later use | | Detection hit + user picks `not applicable` | Reason recorded;close completes cleanly | -| `gh api PATCH` fails (network / auth) | Print warning;close still proceeds (audit trail loss is non-blocking) | -| Plugin name resolution fails (marketplace.json malformed) | Treat as `n/a` + audit line `(detection error: marketplace.json parse failed)` | +| `gh api PATCH` fails (network / auth) | `patch_closing_comment_append` prints warning to stderr;close still proceeds (audit trail loss is non-blocking) | +| Plugin name resolution fails (marketplace.json missing/malformed) | Demote `DISTRIBUTION_TYPE` to `n/a` + WARN to stderr;close completes via standard `n/a` path (no separate audit error category — keeps schema simple) | > **Future hook for `idd-implement` Step 5.x early-surface**: same `infer_distribution_type` helper could power an earlier prompt (at PR-open time rather than close time). Out of scope for #45 v1 — separate follow-up if value confirmed. From 9e196d8e7b9927b5357d31dad090c6df4028d097 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 10 May 2026 19:42:09 +0800 Subject: [PATCH 6/6] docs(idd-close): A1+A3 P3 doc fixes from round-3 verify (Refs #45) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A1 — failure-mode REPO_ROOT row: clarify pwd fallback covers non-git case; unresolvable branch fires only when both git rev-parse AND pwd fail. A3 — Extension protocol: removed contradiction ("Don't edit SKILL.md" vs "edit inline implementation in SKILL.md"). Now correctly states two-location update is required since skills don't source reference markdown at runtime. A2 monorepo host disambiguation filed as #68 follow-up (out-of-scope here). --- .../references/distribution-detection.md | 11 ++++++----- plugins/issue-driven-dev/skills/idd-close/SKILL.md | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/issue-driven-dev/references/distribution-detection.md b/plugins/issue-driven-dev/references/distribution-detection.md index e0c546e..6b2d8ab 100644 --- a/plugins/issue-driven-dev/references/distribution-detection.md +++ b/plugins/issue-driven-dev/references/distribution-detection.md @@ -255,12 +255,13 @@ Rationale: CI / batch runs need disable to avoid AskUserQuestion blocking. Patte If a user's wrapper uses unlisted keyword (e.g. `wget`, `npm install`, `brew install`): -1. **Don't edit `idd-close/SKILL.md`** — the SKILL.md delegates to this contract, not inline keywords -2. Edit `has_binary_wrapper` regex in this doc + the inline implementation in `idd-close/SKILL.md` Step 6.5 bash to add the pattern -3. Document the new pattern in this contract under "Detection keywords" section -4. Smoke-test against the repo that has the unusual wrapper +1. **Update the regex in TWO places** (helpers are inlined in skill, not sourced from this doc): + - `has_binary_wrapper` regex in this contract (canonical spec) + - `has_binary_wrapper` regex inlined in `plugins/issue-driven-dev/skills/idd-close/SKILL.md` Step 6.5 detection block +2. Document the new pattern in this contract under "Detection keywords" section +3. Smoke-test against the repo that has the unusual wrapper -**Rationale**: encapsulating detection logic here lets future extension touch one canonical doc rather than hunting through skill prose. +**Rationale**: this contract is the canonical spec — Step 6.5 inlines the algorithm rather than sourcing the doc at runtime (skills don't `source` reference markdown). Two-location update is unavoidable until skills gain a doc-import mechanism;the contract doc is the source of truth that drift checks should compare against. ## Test fixtures (manual smoke matrix) diff --git a/plugins/issue-driven-dev/skills/idd-close/SKILL.md b/plugins/issue-driven-dev/skills/idd-close/SKILL.md index 700b690..3669557 100644 --- a/plugins/issue-driven-dev/skills/idd-close/SKILL.md +++ b/plugins/issue-driven-dev/skills/idd-close/SKILL.md @@ -777,7 +777,7 @@ patch_closing_comment_append "$AUDIT_BLOCK" | Scenario | Behavior | |----------|----------| -| `REPO_ROOT` cannot be resolved (e.g. not in git work tree) | Silent skip + 1-line audit `(REPO_ROOT not resolvable)`;close still completes | +| `REPO_ROOT` cannot be resolved (rare — `pwd` fallback covers non-git case;this branch fires only when both `git rev-parse` AND `pwd` fail or return non-existent dir) | Silent skip + 1-line audit `(REPO_ROOT not resolvable)`;close still completes | | Non-distribution repo (detection returns `n/a`) | Silent skip + 1-line audit `(detection: not a distribution repo)` | | `IDD_DISTRIBUTION_SYNC_PROMPT=false` | Silent skip + 1-line audit citing env var | | Detection hit + user picks `chain` | Skill invoked + outcome appended to audit trail |