cluster: 4 refactor/docs follow-ups from #49 / #60 IDD lifecycles#72
Open
kiki830621 wants to merge 304 commits into
Open
cluster: 4 refactor/docs follow-ups from #49 / #60 IDD lifecycles#72kiki830621 wants to merge 304 commits into
kiki830621 wants to merge 304 commits into
Conversation
Three modes for ensemble-academic-review:
- independent: all reviewers start fresh (default, unchanged)
- hybrid: only devil's advocate sees prior rounds, others remain independent
- mix N: alternates independent → hybrid for N rounds with task tracking
Mix mode design:
- Odd rounds: independent (fresh eyes, can discover new issues + verify prior)
- Even rounds: hybrid (DA sees all prior, hunts blind spots)
- Each round writes review-round-{N}.md independently
- Final merge includes convergence analysis (new findings per round)
- TaskCreate/TaskUpdate tracks progress across rounds
Key principle: methodology, writing, Codex NEVER see prior results
(prevents anchoring bias). Only DA gets full prior context.
4 Claude teammates for teaching material review: - content-accuracy: stats, theory, formulas - student-readability: can the student understand this? - completeness: SRT coverage, structure checks - devils-advocate: challenge all OK judgments
… errors in issues
Previously idd-diagnose directly pointed SDD-warranted issues to /spectra-propose, skipping spectra-discuss. This violated Spectra's official flow (discuss? → propose → apply ⇄ ingest → archive) and produced proposals built on unconfirmed AI assumptions. Changes: - idd-diagnose Step 3.5: spectra-discuss is now the default for SDD-warranted, with explicit opt-out criteria - idd-diagnose Step 4: new two-stage routing (confirm diagnosis correctness → AskUserQuestion forces discuss/propose choice) Commands are displayed, not auto-invoked — user controls pacing - idd-diagnose Next Step section: shows all 3 possible flows - sdd-integration.md: Flow now has default / opt-out paths; added "Why discuss is default" and "When to opt-out" sections; added Rule 5 enforcing discuss-first routing - CLAUDE.md workflow diagram: SDD branch now shows discuss → propose Rationale: AI agents consistently over-estimate diagnosis completeness. spectra-discuss is the alignment safety net that forces assumptions to be stated and corrected before formal proposal. Version: 2.12.3 → 2.13.0 (minor, backward compatible — existing SDD flows still work, just get an extra alignment step by default)
- ensemble-academic-review: use codex exec -o for one-shot review - ensemble-code-review: use codex exec -o, remove status/result polling - idd-verify: unify to codex exec, remove companion fallback chain - docs: add codex-calling-convention.md with exec vs companion guidance
- 修正 CRLF → LF（CRLF 會導致 '/bin/bash^M' 執行失敗）
- GITHUB_REPO 從 che-telegram-{bot,all}-mcp 改為 che-msg（已合併為 monorepo）
- 加 che-msg/ 路徑到 binary fallback 列表（保留舊 che-mcps/ 相容）
當 Claude Code 異常退出時，舊 MCP server process 會 orphan 繼續持有
TDLib database lock，導致下次啟動的新 process 卡在 waitingForParameters。
修正：
- Wrapper 啟動時檢查 ~/.cache/che-telegram-{all,bot}-mcp.pid，若舊 PID
仍活著且 comm name 匹配 BINARY_NAME（PID recycling 防護），先 SIGTERM
+ 0.5s grace period + SIGKILL fallback 清掉
- 改 exec 為 fork + wait + trap，讓 wrapper 退出時能 cleanup PID 檔
並 forward SIGTERM 給 binary
- 兩個 wrapper（bot/all）套用同一 pattern
新增 test-wrapper-pid.sh smoke test，覆蓋 5 個 behavior：
1. PID file lifecycle（建立 / 退出移除）
2. Orphan cleanup
3. PID recycling 防護
4. Stale PID file 處理
5. SIGTERM forwarding
驗證：5/5 passed
從 idd-verify #8 的 6-AI 審查找出的 P1 bugs： 1. cleanup() 順序錯誤 舊版：先 rm PID_FILE 才 kill binary 問題：若 binary 不理 SIGTERM，wait 無限 hang 且 PID file 已消失 → 下次 wrapper 找不到 orphan，原 bug 重現 修正：先 kill binary（含 timeout + SIGKILL fallback）才 rm 2. cleanup() 沒做 ownership check 問題：舊 wrapper 的 trap 晚觸發可能誤刪新 wrapper 的 PID file 修正：rm 前先確認 PID file 內容等於自己的 BIN_PID 3. cleanup 沒有 SIGKILL fallback 問題：binary 卡死時 wrapper 永遠 hang，Claude Code 無法 shutdown 修正：SIGTERM 後 poll 2s，仍存活則 SIGKILL 4. PID recycling 防護用 substring grep 問題：grep -q "CheTelegramAllMCP" 會 match "CheTelegramAllMCPDeluxe" 修正：用 basename + exact match（[[ basename == BINARY_NAME ]]） 5. PID file 解析未驗證格式 修正：tr -d 空白 + regex 驗證為純數字 6. Smoke test flaky（4/5 ↔ 5/5 across runs） 問題：fixed sleep 與 wrapper 內部 0.5s grace period 競爭 修正：改用 wait_for_dead / wait_for_file polling helpers 7. Test 覆蓋率擴充（5 → 7 個 test） - Test 6: ownership check 防止 wrong-owner 刪 PID file - Test 7: SIGKILL fallback 對忽略 SIGTERM 的 binary 仍能 kill 兩個都對應 P1 修正 驗證：5 次連跑 5/5 全部 7/7 passed
從 Round 2 verify 的新發現：
1. tr -d '[:space:]' 把 "12 34" 變 "1234" (Codex 發現)
問題：corrupted PID file 的 internal whitespace 被 coalesce 後可能
變成另一個合法 PID → 誤殺無辜 process
修正：改用 `read -r OLD_PID < $PID_FILE`，preserve 原內容讓 regex
驗證可以正確 reject "12 34" 這類格式
2. wait_for_dead 用 kill -0 偵測 zombie 不可靠 (Codex 發現)
問題：zombie process 的 kill -0 回 success，在 Codex 環境 0/5
deterministic fail (我這邊 10/10 pass 是環境差異)
修正：改用 `ps -p $PID -o state=` 偵測 Z 狀態 + wait reap
3. 加 Test 8: 驗證 corrupted PID file 不被 coalesce
輸入 "12 34"，確認 wrapper 最終寫入的 PID 不是 "1234"
驗證：10 次連跑 10/10 全部 8/8 passed
還沒修的（follow-up）：
- Finding 1 (devils-advocate): Test 7 的 stubborn binary 用 bash trap + sleep，
SIGKILL 只殺 bash 不殺 child sleep。實際 TDLib C++ binary 無此問題。
- Finding 2 (devils-advocate): Init phase SIGTERM window (L61-83 之間)。
機率極低。
- Bot-mcp over-engineering (devils-advocate): bot-mcp stateless，strictly
不需 PID tracking。保留對稱性以降低維護成本。
Wire TargetOptions through 7 deferred commands (#23): - storage local/session × get/set/remove/clear (8 subcommands) - snapshot + snapshot --page - wait (--url renamed to --for-url, BREAKING) - close + screenshot + pdf + upload --native (window-only via WindowOnlyTargetOptions) - upload --js (full TargetOptions) screenshot --window N rewritten to use AXUIElement private SPI (_AXUIElementGetWindow) for reliable AS↔CG window identity. Eliminates the 9-round saga of bounds/title/raise heuristic failure modes. Strict fail-closed: throws windowIdentityAmbiguous on multi-bounds collision instead of silently picking wrong window. Default screenshot (no flag) prefers AX when AXIsProcessTrusted; falls back to legacy CG name match without permission. SKILL.md: new Multi-Window Targeting section with examples. Underlying CLI: 11 fix commits across 9 verify rounds in PsychQuant/safari-browser#23.
Critical bug from Round 1 refactoring (commit 06015c9): 當 wrapper 從 `exec "$BINARY"` 改成 `"$BINARY" "$@" &` 時， POSIX 規則 (POSIX.1-2017 Section 2.9.3.1) 在非互動 shell 下會把 asynchronous command 的 stdin 自動重導到 /dev/null。這切掉了 MCP server 的 JSON-RPC stdio protocol，binary 讀 /dev/null 立刻 EOF → segfault。 6 輪 verify + 8 個 smoke test 全部漏掉，因為 fake binary 是 /bin/sleep， sleep 根本不讀 stdin，這個 bug 路徑從未被執行。 修正： - 兩個 wrapper 改為 `"$BINARY" "$@" <&0 &`，顯式繼承 stdin fd 0 - Smoke test 加 Test 0：用 /bin/cat 當 fake binary（會讀 stdin） 驗證 stdin 確實被 inherited。未來任何破壞 stdin 的改動都會被 catch - make_fake_wrapper 內部也加 `<&0` 對齊 production pattern 驗證： - 手動 echo '{"jsonrpc":"2.0",...}' | wrapper → binary 正常回應 - 5 次連跑 smoke test 全部 9/9 passed - Test 0 是 regression test：若未來有人移除 `<&0` 會立刻 fail bump: v1.0.2 → v1.0.3
舊格式 {"hooks": [{type, matcher, ...}]} 會讓 /reload-plugins 報
"expected record, received array"。Claude Code 的 hooks schema 期待
以 event name 為 key 的 object：
{
"hooks": {
"PostToolUse": [
{ "matcher": "Bash", "hooks": [...] }
]
}
}
參考其他正常載入的 plugin（archive-first、lean-prover、che-ical-mcp）
改寫成同樣格式。
#8 為了解決 all-mcp 的 TDLib DB lock orphan 問題，加了 PID tracking + ownership check + SIGKILL fallback。同樣的複雜度被無腦 copy 到 bot-mcp，純粹為了「對稱性」。 但 bot-mcp 是 stateless HTTPS client： - 沒有 local DB - 沒有 file lock - 沒有 single-instance constraint - 沒有 orphan 卡住下次啟動的問題 整套 25 行的 PID 邏輯對 bot-mcp 都是 dead code，反而會製造 #10 的 multi-instance race（兩個 wrapper 互殺）。 修正：bot-mcp wrapper 簡化回單行 exec "$BINARY" "$@"，加長註解 說明為什麼 bot 不需要 PID tracking（避免未來有人「順手對齊」加回來）。 驗證：smoke test 9/9 passed (test 用 fake binary，不依賴 bot wrapper)
…heck (#11) 原版 stubborn binary (#8 Round 2 加的)： #!/bin/bash trap '' TERM sleep 30 bash 確實 ignore SIGTERM，但 `sleep 30` 是 child process。Wrapper SIGKILL bash subshell 時，sleep 變成 orphan 被 launchd 接管繼續跑。Test 只檢查 bash PID 就 PASS，沒偵測到 orphan child — test 的語意與實作不一致。 修正： 1. Stubborn binary 改單 process：用 `read -t 30 < /dev/null` bash builtin 取代 `sleep 30`（bash builtin 不 fork child） 2. Test 7 加第二個 assertion：snapshot `pgrep -P $STUBBORN_PID` 記錄 binary 的 children，SIGKILL 後驗證所有 children 都死掉 3. 順便對齊 wrapper7.sh 的 ownership-check pattern 為 #9 的 `read -r` 風格（取代 `tr -d '[:space:]'`） 驗證：5 次連跑全部 9/9 passed。Test 7 現在真的測到它宣稱的東西。 Production impact: 無 — CheTelegramAllMCP 是單一 Swift binary，沒有 child process 結構，不會踩到這個 orphan 問題。但 test methodology 改進後，未來若 MCP server 換成多進程架構，這個 regression 會被 catch。
Verify round found two legitimate issues: ### Logic P0 (#11): stubborn binary exited immediately (test was false positive) \`read -t 30 < /dev/null\` 讀 /dev/null 立刻收到 EOF，返回 exit=1，根本 沒 block 30 秒。Test 7 的 wrapper SIGTERM → 2s wait → SIGKILL 路徑 完全沒跑到，binary 自己退出 test 就 pass — 這是 false positive。 實測（手動 bash -c 'read -t 30 < /dev/null'）秒返。 修正：用 FIFO + read 讓 bash 主程序 block 在 open() 系統呼叫。 核心知識：open FIFO for read 會 block 直到有 writer 或 interrupt， 沒有 writer 就 block 到 -t 30 timeout，且不 fork 任何 child。 驗證：Test 7 現在總 runtime 從 ~1s 拉到 2-3s（SIGKILL path 真的跑到）， 5 次連跑全部 9/9 passed。 ### Devil's Advocate HIGH (#12): bot-mcp getUpdates long-polling conflict 原本的 comment 說「多個 bot-mcp instance 並存對 Telegram 服務端來說 是合法的」— 這個陳述 partially false。Telegram Bot API 規定同一個 bot token 只能有一個 client 在 long-polling getUpdates，第二個會 得到 409 Conflict 錯誤。 修正註解：明確區分「local resource」和「server-side polling」。 Local 端確實沒 PID tracking 需求；server 端 getUpdates 衝突應該在 Server.swift 用 advisory lock 處理，不是 wrapper 的責任。 實質 code 不變（wrapper 還是 exec），但 claim 誠實多了。 Files: - che-telegram-bot-mcp-wrapper.sh: amend NOTE comment - test-wrapper-pid.sh: stubborn binary 改 FIFO-based blocking read
…y runs (#11) Codex verify found that previous amendment had race between wrapper setting up trap+FIFO and test harness sending SIGTERM. Measured wrapper exit time: - 500ms: bash died before trap installed → SIGKILL never hit - 2000ms: trap held → wrapper waited → SIGKILL fired False-positive rate was ~60% in Codex's 10-run tests. Three fixes: 1. Add sentinel file ($STUBBORN_READY) — stubborn binary touches it AFTER `trap '' TERM` AND `mkfifo`. Test waits for this file before sending SIGTERM, guaranteeing trap is active. 2. Error-check `mkfifo`: previous version silently swallowed failure. 3. Add timing assertion: measure elapsed time from SIGTERM to wrapper exit, assert >= 1500ms. If elapsed < 1500ms, we know SIGKILL path was not exercised (bash died before trap took effect). 4. Remove `-t 30` from read — bash's -t doesn't apply during open() syscall on FIFO anyway. Comment updated to reflect reality. 5. Add EXIT trap in stubborn binary to clean up FIFO on normal exit (SIGKILL won't run it; leftover FIFOs in $TMPDIR are harmless). 驗證：5 次連跑，每次 2064-2120ms elapsed，確認 SIGKILL path 每次都跑到。 9/9 passed, deterministic.
新 skills： - idd-comment: template-guided comment (decision/note/question/correction/link/errata 6 types) - 強制 blockquote 原文、加 timestamp、HTML metadata marker - errata type 會自動 call idd-edit 在 target comment 加警示 - idd-edit: 編輯既有 comment (append/replace/prepend-note 3 modes) - 解決 `gh api PATCH` 手動 escape bug（用 -F body=@file） - 必 show 原 body backup + preview 讓 user confirm - 支援 comment:<id> 和 #issue --last 兩種 target 語法 附帶： - idd-implement: 新增「每張圖下方必須附資料/統計/結論」鐵律 - CLAUDE.md: 分「流程 skills」和「輔助 skills」兩組呈現 - marketplace.json + plugin.json: version 2.13.0 → 2.14.0
- Remove 3 PII instances of d06227105@ntu.edu.tw, replace with user@example.com (#15 MEDIUM) - Expand #15 warning: mark as global (Step 3+5), add explicit reminder that get_email reuses account_name - Drop stale allowed-tools prefixes (mcp__apple-mail__*, mcp__che-apple-mail-mcp__*), only keep plugin prefix (#14 P3) - Filename rule fixes (#16): - Remove "merge consecutive dashes" rule (contradicts all examples and historical files) - Define 50-char limit as Unicode code points (Swift String.count), not bytes - Add no-subject fallback for empty/all-punctuation subjects - Re-trim trailing dashes after truncation - Fix off-by-one in suffix rule: 1st=no suffix, 2nd=-1, 3rd=-2 (example updated to 4th="-3") - Add Glob-based detection for next suffix number - Note on -a/-b legacy files: keep as-is, no migration - Update Step 6 JSON index example filename to new format - Update Step 7 output report examples to YYYY-MM-DD_* format Fixes verify findings on #14 #15 #16
…20) Verify finding: "# Zero-arg mode (planned in #13)" as a comment inside an active code block misleads readers into thinking the zero-arg call is available today. Move zero-arg documentation out of the code block into a dedicated "Roadmap" blockquote with an explicit "not yet implemented" label. Fixes verify P2 on #20
- #16 P1: Swift String.count is grapheme cluster count, not Unicode code point (previous wording conflated the two). Correct the spec wording. - #15 P2: Duplicate the account_name warning into Step 5 so the EWS trap is visible at the point where get_email is actually called. Also make explicit that search_emails does not return content, so get_email is a required step (not an implicit detail). Fixes re-verify partials on #16 #15
docs(che-apple-mail-mcp): fix command spec docs + add README Commands section
- decision / note / correction templates 強制論述段落
- 原 {body} 改為 {body_prose} 明示 prose 要求
- correction 新增「論述 / Supporting evidence / Related」三段結構
- note 新增「論述 / Details」分離 prose vs bullets
- 鐵律加「第一段必 prose」：避免 bullet-only 隱藏邏輯 gap
- 動機：real-world feedback 發現初版 correction comment 全 bullets
三個月後回讀無法重建推論鏈條
- marketplace.json 描述加「prose-first」副標
- Bump plugin to 2.1.1 to match MCP server release (fixes #9 in che-apple-mail-mcp) - Update wrapper REPO to PsychQuant/che-apple-mail-mcp (repo moved from kiki830621)
…ling-archive dedup (#49) Workspace Layout Detection (v2.17.0):當 output_dir 既無 ARGUMENTS[1] 也 無 config output_dir 給定時,probe communications/email/ → correspondence/ emails/ → default communication/emails 順序解析。Detection-first not prescriptive — adapt 到 user 既有 layout,不 push canonical convention。 有 explicit config 的 workspace 行為 100% backward-compat。 Sibling-archive dedup extension:output_dir 下 symlinked subdirectory (transitioned-project pattern,如 chchen_lab email/application/ → applications/ completed/.../emails/)的 markdown YAML frontmatter 自動讀 message_id: 併入 in-memory dedup set。find -P -maxdepth 2,read-only。Composes with dedup_strategy=index|both (skip on last_archived)。 Ambiguity guard:communications/email/ 與 correspondence/emails/ 同時 存在且都有 *.md 時 abort with explicit pin recommendation,避免 mid- migration workspace 的 dedup-index split-brain。 Three-way version sync verified:plugin.json + marketplace.json + CHANGELOG.md 全部 2.17.0。description 在 plugin.json ↔ marketplace.json 對齊。 Plan-tier path: /idd-plan #49 EnterPlanMode → user-approved → /idd-implement (direct-commit, --no-pr per user choice in Phase 0.5)。 Files changed: - plugins/che-apple-mail-mcp/commands/archive-mail.md (Step 1 detection cascade + Step 2.1 dedup extension) - plugins/che-apple-mail-mcp/README.md (Workspace Patterns v2.17.0+ section) - plugins/che-apple-mail-mcp/.claude-plugin/plugin.json (2.16.1 → 2.17.0) - plugins/che-apple-mail-mcp/CHANGELOG.md ([2.17.0] entry) - .claude-plugin/marketplace.json (sync 2.17.0 + description) Out-of-scope follow-ups already filed: #50 (documents_dir parallel detection), #51 (companion-commands consistency)。 Refs #49
對應 GiftHub CLI v0.4.0 把 default config 改 .claude/.gfs/ subdir。 Plugin v1.2 同步 hook + skill 兩 layout 並陳。 ## 影響檔案 - hooks/session-start.sh: detect v2 (.claude/.gfs/config.json) 優先 / v1 (.gfh.json) fallback；inject context 含 Layout: v1|v2 (path) 標示 - skills/gfh-import/SKILL.md: Prerequisites + Step 1 read config 用 v2 priority chain - .claude-plugin/plugin.json: bump 1.1.0 → 1.2.0；description 同步 - CLAUDE.md: 加 Config layout 表格說明 v1/v2 - CHANGELOG.md: v1.2.0 entry ## Backward-compat v1 repo 仍 work，consumer 不需特定升級順序（gfh CLI 與 plugin 各自獨立）。 Refs #59 Coordinated: PsychQuant/GiftHub#17
…sychQuant/che-apple-mail-mcp#76) (#61) Wrapper writes runtime state on spawn; new session-start hook compares against plugin.json version and SIGTERMs stale MCP PIDs so Claude Code respawns with the new in-memory binary. - bin/che-apple-mail-mcp-wrapper.sh: atomic write ~/bin/.CheAppleMailMCP.runtime.json (pid/started_at/version_at_spawn) before exec, silent skip on failure - hooks/session-start.sh (new): version-mismatch + alive PID + command-field guard ⇒ SIGTERM (5s grace) + SIGKILL fallback; full graceful-skip on jq/ps/file/PID-dead/command-mismatch - tests/test-session-start-hook.sh (new): 6 case integration test, 16/16 PASS using exec -a CheAppleMailMCP-mock sleep - plugin.json: bump 2.17.0 → 2.18.0 + description - CHANGELOG: v2.18.0 entry with sister-issue cross-link Sister issue split: #58 (plugin-update should warn user about stale MCP PIDs — sender-side complement).
…n gate (#60) Phase 0.5 inserts between Phase 0 (detect marketplace) and Phase 1 (detect changes). Read-only preview block shows git status / unpushed commits / divergence; AskUserQuestion 5-case dispatch: - Case A clean+0 unpushed → abort with "nothing to push" - Case B clean+N unpushed → default "push N as-is" - Case C dirty+0 unpushed → default "abort" (user manually stages) - Case D dirty+N unpushed → default "abort" (4 sensible actions, user picks) - Case E origin diverged → default "abort" (user resolves conflict) Default option = abort for any state with multiple sensible actions; only unambiguous clean+unpushed defaults to push. idd-all unattended mode: auto-abort with structured error + non-zero exit. Same pattern as idd-diagnose Step 3.4 F unattended handler. Phase 1 Step 2 (existing narrative advisory "請先 commit + push" without gate) replaced with one-liner reference to Phase 0.5. AI executor previously could print reminder + continue; v1.16.0 forces explicit AskUserQuestion. Cross-plugin commits warn-only (Tier A scope) — heads-up print if unpushed commits touch >1 plugin, but no refuse. Active scope guard deferred to #65. Step 0 stage TaskList adds git_state_gate entry between detect_marketplace and detect_changes. Three-way version sync verified: - plugins/plugin-tools/.claude-plugin/plugin.json 1.15.0 → 1.16.0 - .claude-plugin/marketplace.json plugin-tools entry → 1.16.0 - CHANGELOG.md [1.16.0] - 2026-05-10 entry added - description fields byte-identical between plugin.json and marketplace.json Tier A only this PR: Tier B (commit message auto-draft) + Tier C (active cross-plugin scope guard) deferred until Tier A UX validated. Plan-tier path: /idd-plan #60 EnterPlanMode → user-approved → /idd-implement (direct-commit, --no-pr per user choice in Phase 0.5). Files changed: - plugins/plugin-tools/skills/plugin-update/SKILL.md (Phase 0.5 insert + Phase 1 Step 2 deprecate) - plugins/plugin-tools/CHANGELOG.md ([1.16.0] entry) - plugins/plugin-tools/.claude-plugin/plugin.json (1.15.0 → 1.16.0) - .claude-plugin/marketplace.json (sync 1.16.0 + description) Out-of-scope follow-ups already filed: #63 (plugin-deploy/plugin-create sibling rollout), #65 (mcp-deploy/cli-deploy cross-marketplace), #64 (pre-existing CHANGELOG [1.15.0] placeholder date drift), #58 (post- marketplace-sync stale-MCP-PID warning, complementary lifecycle angle). Refs #60
…ack parity + cluster #61-64 hardening)
Verify report (#60 (comment)) returned FAIL with 4 BLOCKING + 3 IN-SCOPE-FIX. This commit applies all 7 fixes. P1 BLOCKING #1 — `@{u}` upstream silent fallback (Codex P1 + Logic P0-1 + Security LOW): Replace fragile `git rev-parse ... | sed 's|^origin/||' || echo main` chain with explicit upfront UPSTREAM resolution. Aborts with structured error on no-upstream / detached-HEAD / incomplete-rebase, matching L128 spec contract instead of silently proxying to origin/main. P1 BLOCKING #2 — 5-state disjoint not enforced (Codex P1 + Logic P1-1+P1-2): Replace ambiguous detection table with priority-ordered table. Divergence (priority 1) tested first; dirty/clean × unpushed (priority 3-6) tested only when divergence == 0,0. New Case E' (pure-behind, M≥1 N=0) explicitly added — was missing from original draft, would have mis-routed to fetch+rebase prompt. P1 BLOCKING #3 — Case D amend missing `git add -A` (Codex P2, DA→P1): Option description claimed "amend dirty into HEAD" but bash only ran `git commit --amend --no-edit` which doesn't pick up unstaged changes. Now prefixes with `git add -A` so dirty actually gets merged into HEAD as promised. P1 BLOCKING #4 — TARGET_PLUGIN ordering bug (Codex P2, DA→P1): Phase 0.5 Step 5 used `TARGET_PLUGIN=$1` but Phase 1 Step 1 supports plugin name inference when $1 is empty. Phase 0.5 ran before Phase 1's inference, causing single-plugin commits to false-warn. New version checks if TARGET_PLUGIN is set, skips silently if not (deferring scope check until Phase 1 inference completes). P2 IN-SCOPE-FIX #5 — cross-plugin warn empty-set hole (Codex P2 + Logic P0-2): Original warn logic had no branch for "0 plugins touched" (commits only modify root files / docs / empty commits). Most dangerous case (running /plugin-update foo when commits don't touch plugins/foo/) silently passed. New 3-case structure: empty / happy / cross-plugin, all with appropriate messaging. P2 IN-SCOPE-FIX #6 — trailing whitespace (Codex `git diff --check`): Stripped trailing whitespace from SKILL.md (BSD sed in-place). Lint hygiene. Note: large diff churn because sed normalized entire file, not just Phase 0.5 insert region — accepting one-shot whitespace cleanup as bonus. P3 IN-SCOPE-FIX #7 — stale line number references (Codex P3): Replaced "line 161" / "line 437" textual refs with anchor links to Phase 1.5 / Phase 2.5 section names. Phase 0.5 insert shifted everything ~190 lines so old refs were broken. New section refs are insert-resilient. Out-of-scope follow-ups filed (not in this commit): - #69: IDD_ALL_UNATTENDED env var contract across idd-all + plugin-tools - #70: /tmp race hardening (use mktemp instead of hardcoded path) — LOW - #64 scope expanded (via comment) to include description backfill Re-verify deliberately skipped per user choice. Fixes are content / pseudocode edits to same skill body, no new logic introduced. Parallel session committed v2.18.1 between original `0552ecf`-equivalent and this fix → not amend, NEW commit so as not to touch concurrent release commit `11bb925`. Refs #60
…ychQuant/che-apple-mail-mcp#77) The wrapper sidecar (~/bin/.CheAppleMailMCP.version) was recording the plugin SHELL version (from plugin.json `version`, e.g. "2.18.1") instead of the actual downloaded BINARY tag (e.g. "2.7.2"). On the next spawn, INSTALLED_VERSION == DESIRED_VERSION matched even though the binary at GitHub releases/latest had moved forward — silent skip of binary-only releases. Two fixes: 1. **plugin.json adds explicit `binary_version` field** ("2.7.2"). This disambiguates from the plugin shell's own `version` (2.18.1). Wrapper reads it preferentially; falls back to `version` for plugins that haven't migrated. 2. **Wrapper writes the ACTUAL downloaded binary tag to the sidecar**, parsed from the GitHub release URL path segment between `/releases/download/` and the next `/`. Even when DESIRED is wrong (legacy plugins reading shell `version`), the sidecar now honestly reflects what's on disk — making the next compare honest instead of structurally lying. Smoke tested: wrapper bash syntax OK, plugin.json valid JSON, binary_version field extracts cleanly, URL tag parser pulls "2.7.2" from the canonical download URL pattern. Refs PsychQuant/che-apple-mail-mcp#77
* docs(che-apple-mail-mcp): backfill CHANGELOG [2.16.1] entry (Refs #52) Original v2.16.1 release (commit 8089765, 2026-05-09) bump-only-shipped binary v2.7.1 but skipped the CHANGELOG entry. KAC 1.1.0 invariant requires every released version to have an entry. Backfilled section uses ### Notes (bump-only, no shell-side Added/Changed/Fixed): - Binary v2.7.1 ships: #72 base64 fix, #69 SQLite fast-path stderr logging, #66 .partial.emlx attachment fix - Tool count drift catch-up 44 → 47 in user-facing surfaces Inserted between [2.17.0] and [2.16.0] in chronological release order. Refs #52 * docs(archive-mail): reduce 'schema diversity' claim to v2.6+ format compat (Refs #54) Per /idd-verify #49 logic F5 + codex P3 + DA verification: original 'Resilient to schema diversity' claim was overpromise. archive-mail v2.6+ self-archived files all use double-quoted message_id format; manual / external archives with single quotes / inline comments / CRLF / trailing whitespace would silent-skip without warning. Option A.1 (claim reduction) per issue body: - Replace claim with 'v2.6+ format compatibility' framing - Document silent-skip behavior for non-conforming frontmatter - Defer parser hardening (yq / robust awk) until N≥3 user reports No code change — pure docs honesty fix. Refs #54 * docs(che-apple-mail-mcp): soften '100% backward compat' Workspace Patterns wording (Refs #55) Per /idd-verify #49 DA new finding: '100%' was technically incorrect — zero-config mid-migration workspaces with both communications/email/ and correspondence/emails/ populated will trigger ambiguity guard abort, which IS a backward-compat change from pre-v2.17 silently-pick-default behavior. Replace strong '100%' claim with explicit-pin / mid-migration distinction: - Explicit-pin workspaces: behavior unchanged (true) - Zero-config + multi-layout: ambiguity guard flags for explicit pin Honest framing matches the actual gate behavior shipped in v2.17.0. Refs #55 * docs(plugin-tools): backfill CHANGELOG [1.15.0] date + restore description detail (Refs #64) Original v1.15.0 entry shipped (2026-04-26 commit 60f8cab) but used changelog-tools:changelog-init placeholder '(date unknown — please fill in)' that was never filled. Per scope expansion comment on #64, also backfill the v1.15.0 feature detail (was previously only in marketplace.json description but lost when v1.16.0 description replaced it). Date 2026-04-26 verified via 'git show -s 60f8cab'. Description content extracted from commit body (s4/s5/s6 detector signals + 2 suppressions) + original marketplace.json description (cross-28-plugin audit pattern). KAC ISO-format requirement now satisfied; changelog-validate strict mode no longer flags this entry. Refs #64 * fix(docs-cluster): apply /idd-verify PR #71 codex findings (Refs #52 #54 #64) Codex GPT-5.5 cluster verify returned FAIL with substantive findings; empirical adjudication kept 3 of 5 (rejecting 2 false positives on KAC section names — '### Notes' is established repo convention used 6× in che-apple-mail-mcp/CHANGELOG.md and across batch-ocr/plugin-tools/ parallel-ai-agents). Applied 3 fixes: - #54 MEDIUM (UPHELD): archive-mail.md L382 properties block — corrected parser failure-mode description. Original wording said 'silent skip with ENTRIES_THIS_DIR=0' but empirical awk test confirms parser produces non-empty malformed key (e.g. \"'<abc>'\" with single quotes preserved when input is single-quoted), so ENTRIES_THIS_DIR increments with garbage key → dedup MISS not skip. New wording explicitly describes the malformed-key dedup-miss risk. - #52 LOW (UPHELD): che-apple-mail-mcp CHANGELOG [2.16.1] entry — narrowed 'tool count drift catch-up in user-facing surfaces' to 'plugin.json + marketplace.json descriptions' since commit 8089765 only touched those 2 files (README untouched). Added note that README sync should be follow-up work. - #64 LOW (UPHELD): plugin-tools CHANGELOG [1.15.0] suppression B — broadened from narrow 'bin/*-wrapper.sh only' to full set per 60f8cab commit body: wrapper-only / marketplace-sync / plugin.json version field. Includes commit message patterns + path patterns + 1a4c1fe canonical example. Rejected: - #52 HIGH '### Notes' non-KAC: empirical 6× usage in this CHANGELOG + presence in batch-ocr/plugin-tools/parallel-ai-agents establishes '### Notes' as repo convention. Codex over-strict KAC reading. - #64 HIGH same as above. Refs #52 #54 #64
…p 2.1 (Refs #56) Per /idd-verify #49 DA new finding: Step 2.1 sibling-archive dedup extension claims 'read-only' but enforcement is honor-system (current bash uses only find/head/awk; future maintainer adding mv/rm wouldn't be caught by any guard). Option A per issue body — invariant marker comment at top of bash block: - Lists allowed ops (find / head / awk / cat / grep) - Lists forbidden ops (mv / rm / cp / > / >> / tee / chmod) - Self-documenting, load-bearing contract for future maintainers Option B (bash trap-based check) + Option C (pre-commit hook scanning) deferred — requires real bash impl file or repo-level tooling, both out of scope for this docs-only fix. Refs #56
Per /idd-verify #49 security review LOW#1: Step 2.1 sibling-archive dedup extension used 'find ... | while IFS= read -r' which splits on newlines. Filenames containing newlines (POSIX-legal, rare in practice) would silently mis-parse — partial filename gets processed. Apply standard null-safe pattern to BOTH find invocations: - Outer: find -P -type l -print0 + while read -r -d '' symlink_dir - Inner: find -P -name '*.md' -print0 + while read -r -d '' mdfile Both -print0 / read -d '' must change together — partial conversion of one without the other would mis-parse the pipeline. Realistic exposure low (Apple Mail subject lines don't contain raw newlines), but trivial defensive hygiene fix. Refs #57
…nment with Phase 0.5 (Refs #68) Per /idd-implement #60 Step 5.7 sister bug sweep: audit found Phase 1.5 (action default '順便更新') + Phase 2.5 (action default '更新 README') appear to diverge from Phase 0.5's abort-default safety policy. Audit conclusion: NOT a divergence — they correctly apply Phase 0.5 rule's exception clause: 'active default is reserved for unambiguous happy-path / reversible action / frequent dev flow' Phase 1.5: binary-out-of-sync in daily dev flow IS unambiguous happy path. Binary install is reversible (mcp-deploy / cli-upgrade can reset). Frequent. → action default justified. Phase 2.5: '更新 README' option doesn't mutate; it proposes diff (read CHANGELOG + git log → draft → user reviews → commit). User- confirmation gate still present. Reversible draft. → action default justified. Contrast with Phase 0.5 (release-time push to public marketplace): push is IRREVERSIBLE (force-push to public is bad day) → abort default warranted regardless of user-in-session. Each phase's default reflects its action's risk profile. No behavior change — just documentation that the deliberate divergence IS the correct application of Phase 0.5 rule, not a violation. Refs #68
….5 Step 5 (Refs #70) Per /idd-verify #60 security review (LOW, DA→LOW): finding L233 flagged '/tmp/touched-plugins.txt race / TOCTOU on shared /tmp'. Audit found the flagged tempfile DOES NOT EXIST in current implementation — Phase 0.5 Step 5 already uses inline subshell: TOUCHED=$(git log --name-only --pretty=format: ... | sort -u) Inline $(...) substitution captures to a process-local variable, never touches /tmp, hence no TOCTOU window and no race. The verify finding was a false positive imagining a tempfile that was never written. No behavior change. Add load-bearing comment documenting: 1. Why current code is already safe (inline subshell, not tempfile) 2. If future refactor *does* need persistence, use mktemp + trap pattern (the '#70 expected behavior' from the issue) 3. Hardcoded /tmp/touched-plugins.txt is FORBIDDEN — it would re-introduce the false-positive surface and the actual race window Comment forestalls future audits from re-flagging the same false positive, and gives future maintainers explicit guidance if refactoring direction changes. Refs #70
This was referenced May 10, 2026
…e exception + explicitly justify frequent-dev → non-destructive substitution (Refs #68) Per /idd-verify --pr 72 codex MEDIUM finding (single blocking): Phase 2.5 audit blockquote silently substituted 'non-destructive' for 'frequent dev' in the 3-criterion exception clause, while Phase 1.5 quoted all 3 verbatim. Codex (gpt-5.5 xhigh) flagged the inconsistency: Phase 1.5 note: 'unambiguous happy-path / reversible / frequent dev flow' ✓ Phase 2.5 note: 'unambiguous happy-path / reversible / non-destructive' ✗ ^^^^^^^^^^^^^^^^ silent substitution Substitution is defensible (README updates aren't a daily dev flow — they fire only after substantive change), but the audit must EXPLAIN the substitution, not silently reword the canonical rule. Otherwise future readers comparing the two notes would see two different rule statements and conclude the policy is incoherent. Fix: rewrite Phase 2.5 note as 3-bullet itemize listing each criterion: - unambiguous happy-path ✓ (with explanation) - reversible action ✓ (with explanation — propose diff, not mutate) - frequent dev flow ✗ → substituted with non-destructive (with reason) Phase 1.5 note unchanged (all 3 criteria fit verbatim). Refs #68
Member
Author
Verify Report — PR #72EngineLighter cluster ensemble (1 combined Claude reviewer covering Requirements/Logic/Security/Regression/Devil's-advocate + 1 Codex GPT-5.5 xhigh independent). Pure-docs/refactor cluster — no behavior change, 90-line diff. AggregatePASS (after Scope coveragePR refs: #56 #57 #68 #70 #56 — invariant marker comment (archive-mail Step 2.1)Coverage: FULLY (both reviewers agree)
Devil's advocate:
#57 — null-safe find/while (archive-mail Step 2.1)Coverage: FULLY (both reviewers agree)
Devil's advocate: bash-specific (dash doesn't support #68 — Phase 1.5/2.5 default-option policy alignment auditCoverage: FULLY after fix
Fix applied (
The substitution is now explicit and justified rather than silent. Phase 1.5 note unchanged.
#70 — Phase 0.5 Step 5 tempfile-avoidance commentCoverage: FULLY (both reviewers agree)
Devil's advocate: trap example is documentation, not enforcement. Acceptable — matches #70's "comment guardrail" scope. Cross-cutting findingsScope / regression
KAC/CHANGELOG
Codex disagreement → fix
Severity-tagged findings list (post-fix)
No BLOCKING / HIGH / MEDIUM remaining. Routing
What this verify deliberately did NOT do
|
Bump plugin shell v2.18.1 → v2.19.0 to ship binary v2.8.0 (just released at PsychQuant/che-apple-mail-mcp#85, tagged v2.8.0). Binary v2.8.0 contents: - (#19) sanitize_links opt-in URL scheme allowlist for markdown mode - (#85) formal spec.md Requirement+Scenarios + builder-layer wiring contract tests pinning sanitizeLinks forwarding - (#20/#21/#25/#27/#32) cluster A hygiene + invariant tests - (#73) extractHTMLBody base64+UTF-8-QP decoding fixes - 47 → 48 tools (sanitize_links param surface adds new schema field) - swift test 309 → 313 passing / 0 failures / 8 skipped Files synced: - plugins/che-apple-mail-mcp/.claude-plugin/plugin.json (version + binary_version + description) - .claude-plugin/marketplace.json (same fields mirrored from plugin.json) - plugins/che-apple-mail-mcp/README.md (Version History entry added) Refs PsychQuant/che-apple-mail-mcp#19 #85 #73
This was referenced May 10, 2026
kiki830621
added a commit
that referenced
this pull request
Jul 5, 2026
v3.16.0 ships Bundle B (Refs #70 #71 #72): - #71 silent priority on conflicting anchors → structured error - #72 explicit text_instance ≤ 0 rejected - #70 tool-prefix on 32 return Error: lines in 4 #61-target tools BREAKING (input validation only) — minor bump per SemVer rationale (no schema break, no tool removal, restricting previously-undefined behavior). Suite: 201 → 226 (0 fail / 9 skip).
kiki830621
added a commit
that referenced
this pull request
Jul 5, 2026
kiki830621
added a commit
that referenced
this pull request
Jul 5, 2026
* docs(che-apple-mail-mcp): backfill CHANGELOG [2.16.1] entry (Refs #52) Original v2.16.1 release (commit 7513a5d, 2026-05-09) bump-only-shipped binary v2.7.1 but skipped the CHANGELOG entry. KAC 1.1.0 invariant requires every released version to have an entry. Backfilled section uses ### Notes (bump-only, no shell-side Added/Changed/Fixed): - Binary v2.7.1 ships: #72 base64 fix, #69 SQLite fast-path stderr logging, #66 .partial.emlx attachment fix - Tool count drift catch-up 44 → 47 in user-facing surfaces Inserted between [2.17.0] and [2.16.0] in chronological release order. Refs #52 * docs(archive-mail): reduce 'schema diversity' claim to v2.6+ format compat (Refs #54) Per /idd-verify #49 logic F5 + codex P3 + DA verification: original 'Resilient to schema diversity' claim was overpromise. archive-mail v2.6+ self-archived files all use double-quoted message_id format; manual / external archives with single quotes / inline comments / CRLF / trailing whitespace would silent-skip without warning. Option A.1 (claim reduction) per issue body: - Replace claim with 'v2.6+ format compatibility' framing - Document silent-skip behavior for non-conforming frontmatter - Defer parser hardening (yq / robust awk) until N≥3 user reports No code change — pure docs honesty fix. Refs #54 * docs(che-apple-mail-mcp): soften '100% backward compat' Workspace Patterns wording (Refs #55) Per /idd-verify #49 DA new finding: '100%' was technically incorrect — zero-config mid-migration workspaces with both communications/email/ and correspondence/emails/ populated will trigger ambiguity guard abort, which IS a backward-compat change from pre-v2.17 silently-pick-default behavior. Replace strong '100%' claim with explicit-pin / mid-migration distinction: - Explicit-pin workspaces: behavior unchanged (true) - Zero-config + multi-layout: ambiguity guard flags for explicit pin Honest framing matches the actual gate behavior shipped in v2.17.0. Refs #55 * docs(plugin-tools): backfill CHANGELOG [1.15.0] date + restore description detail (Refs #64) Original v1.15.0 entry shipped (2026-04-26 commit a55dbe4) but used changelog-tools:changelog-init placeholder '(date unknown — please fill in)' that was never filled. Per scope expansion comment on #64, also backfill the v1.15.0 feature detail (was previously only in marketplace.json description but lost when v1.16.0 description replaced it). Date 2026-04-26 verified via 'git show -s a55dbe4'. Description content extracted from commit body (s4/s5/s6 detector signals + 2 suppressions) + original marketplace.json description (cross-28-plugin audit pattern). KAC ISO-format requirement now satisfied; changelog-validate strict mode no longer flags this entry. Refs #64 * fix(docs-cluster): apply /idd-verify PR #71 codex findings (Refs #52 #54 #64) Codex GPT-5.5 cluster verify returned FAIL with substantive findings; empirical adjudication kept 3 of 5 (rejecting 2 false positives on KAC section names — '### Notes' is established repo convention used 6× in che-apple-mail-mcp/CHANGELOG.md and across batch-ocr/plugin-tools/ parallel-ai-agents). Applied 3 fixes: - #54 MEDIUM (UPHELD): archive-mail.md L382 properties block — corrected parser failure-mode description. Original wording said 'silent skip with ENTRIES_THIS_DIR=0' but empirical awk test confirms parser produces non-empty malformed key (e.g. \"'<abc>'\" with single quotes preserved when input is single-quoted), so ENTRIES_THIS_DIR increments with garbage key → dedup MISS not skip. New wording explicitly describes the malformed-key dedup-miss risk. - #52 LOW (UPHELD): che-apple-mail-mcp CHANGELOG [2.16.1] entry — narrowed 'tool count drift catch-up in user-facing surfaces' to 'plugin.json + marketplace.json descriptions' since commit 7513a5d only touched those 2 files (README untouched). Added note that README sync should be follow-up work. - #64 LOW (UPHELD): plugin-tools CHANGELOG [1.15.0] suppression B — broadened from narrow 'bin/*-wrapper.sh only' to full set per a55dbe4 commit body: wrapper-only / marketplace-sync / plugin.json version field. Includes commit message patterns + path patterns + afc3321 canonical example. Rejected: - #52 HIGH '### Notes' non-KAC: empirical 6× usage in this CHANGELOG + presence in batch-ocr/plugin-tools/parallel-ai-agents establishes '### Notes' as repo convention. Codex over-strict KAC reading. - #64 HIGH same as above. Refs #52 #54 #64
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #56 #57 #68 #70
Summary
Cluster of 4 refactor / docs follow-ups surfaced from
/idd-verifysecurity & sister-bug review of #49 (archive-mail) + #60 (plugin-update). All P3/LOW priority defensive hardening — no behavior change, just invariant markers, null-safe hygiene, and audit-trail comments to forestall future false-positive reviews.Cluster overview
3b01d3f5b610cdf25621eac3c673Per-issue rationale
#56 — archive-mail Step 2.1 read-only invariant marker
Sister-bug from #49 implementation: Step 2.1 sibling-archive-dir dedup extension reads symlinked subdir markdown frontmatter for
message_id:dedup. Block must remain read-only w.r.t.$symlink_dirto preserve immutability invariant. Add invariant marker comment listing allowed (find / head / awk / cat / grep) vs forbidden (mv / rm / cp / > / >> / tee / chmod) operations against$symlink_diror its target. Future trap-based check + pre-commit scan deferred — comment is the load-bearing contract.#57 — archive-mail Step 2.1 null-safe find/while loops
Per /idd-verify #49 security LOW#1: outer
find ... | while IFS= read -r symlink_dirand innerfind ... | while IFS= read -r mdfilesplit on newlines. Filenames containing newlines (POSIX-legal, rare in practice) silently mis-parse — partial filename gets processed.Apply standard null-safe pattern to both:
-print0 | while IFS= read -r -d ''. Both must change together — partial conversion mis-parses pipeline.Realistic exposure low (Apple Mail subject lines don't contain raw newlines), but trivial defensive hygiene fix.
#68 — Phase 1.5/2.5 default-option policy clarification
Sister concern from #60 implementation: Phase 0.5 enforces
abort-default safety policy for ambiguous git states, but Phase 1.5 (順便更新) and Phase 2.5 (更新 README) both have action-defaults. Audit conclusion: NOT a divergence — both correctly apply Phase 0.5 rule's exception clause:Phase 1.5: binary sync in dev flow IS unambiguous happy path; binary install reversible. Phase 2.5: '更新 README' option proposes diff (read CHANGELOG → draft → user reviews → commit), doesn't mutate; user-confirmation gate intact, reversible draft. Phase 0.5 (release-time push to public marketplace) is irreversible → abort default warranted.
No behavior change — documentation that the deliberate divergence IS correct application of Phase 0.5 rule.
#70 — Phase 0.5 Step 5 tempfile-avoidance comment
Per /idd-verify #60 security LOW (DA→LOW): finding L233 flagged
/tmp/touched-plugins.txtrace / TOCTOU. Audit found the flagged tempfile DOES NOT EXIST — Phase 0.5 Step 5 already uses inline subshellTOUCHED=$(git log ...)which captures to process-local variable, never touches /tmp.False-positive verify finding. Add load-bearing comment documenting: (1) why current code is safe (inline subshell, not tempfile), (2) if future refactor needs persistence, use
mktemp + trappattern, (3) hardcoded/tmp/touched-plugins.txtis FORBIDDEN. Forestalls re-flagging by future audits.Verify checklist
Refs #N)/idd-verify --pr <N>against this PR)Generated by /idd-implement #56 #57 #68 #70 --pr cluster-PR mode. Do NOT add
Closes #Ntrailers — IDD discipline requires manual /idd-close per issue after merge to enforce checklist gate + per-issue closing summary.