diff --git a/docs/dev-conventions.md b/docs/dev-conventions.md index b3839c91..cd3ac608 100644 --- a/docs/dev-conventions.md +++ b/docs/dev-conventions.md @@ -126,3 +126,34 @@ pnpm push - **`PR_SIZE_CHECK_OVERRIDE=1`** (push-runner `pr_size_check`): 大型 mechanical refactor (削除≒追加、behavior 不変、test count 不変) で diff が block_threshold を超えるとき。PR description に「mechanical refactor、behavior 不変、test count 不変」を明記する ([PR chain の分割と宣言](#pr-chain-の分割と宣言-adr-069) の項 2 とも整合)。 - **`FILE_LENGTH_CHECK_OVERRIDE=1`** (Stop hook file-length gate、`.claude/hooks-config.toml` `[file_length_gate]`): emergency バイパス専用 (truthy 値で skip、恒久停止は `enabled = false`)。日常運用では使わない — 旧計画書の削除条件 3 (override 未使用で gate 通過が継続) は land 2026-07-02 から 6 週間の全 push で充足を確認し、gate は 2026-08-12 に本採用となった。 + +## 同一事実が複数箇所に分散する場合の変更手順 (順位 445 実装までの暫定 convention) + +1 つの事実 (設定値・routing ポインタ・閾値等) が **code 定数 / config コメント / ADR / facet instruction** など複数箇所に書かれている場合、変更時は**全箇所を 1 つの PR で揃える**。片方だけ直すと、残った側が「古い前提」を語り続け、後続のレビューやレビュアー facet を誤誘導する。 + +1. **変更前に反映先を数え上げる** — `grep` で当該の値・ポインタを含む全ファイルを洗い出し、PR 内で反映先を列挙する (ADR の決定を変えたなら、その決定を引用している別 ADR も対象)。 +2. **「暫定措置」と書いた記述は、恒久化した時点で必ず書き換える** — 条件付き記述 (「〜が確立したら再評価する」) を残したまま条件が消えると、レビュアーが毎回「未文書化の暫定措置」として誤検出する。 +3. **反映先リストを ADR 側に残す** — 次に変更する人が数え上げからやり直さずに済む。 +4. **機械検証できるものは lint へ寄せる** — 本 convention は人手の網羅に依存しており、順位 445 (preamble ⇔ facet routing の集合比較 lint) が入れば少なくとも routing 系は機械側が持つ。**その時点で本 convention の該当部分は撤去する** (ADR-042 のルール vs 仕組み化の境界)。 +5. **1 PR に収まらない場合は PR chain で分ける** ([ADR-069](adr/adr-069-pr-chain-declaration.md)) — 反映先が多く size gate (block 1500 行) に当たる場合、「1 PR で揃える」を守るために無理な圧縮をしない。良い切断点があれば chain に分け、**chain 全体で全反映先を更新すること**と**各 PR の担当範囲**を先頭 PR の計画文書で宣言する。良い関節が無ければ `PR_SIZE_CHECK_OVERRIDE=1` + 理由の明記が正当 (ADR-069 § 2 と同じ判断基準)。**分けてよいのは反映先の集合であって、一部を「後で直す」ことではない** — 中間状態で古い前提が残る期間を作らないよう、chain の順序は「古い前提を参照している側を先に land する」向きに取る。 + +**由来** (2026-08-13、PR #395 / #396 の post-merge feedback で独立に 2 件観測): (a) `docs/todo.md` preamble の routing 更新に対し `.takt/facets/instructions/review-todo-whole.md` の固定値 (`todo6.md` / `todo2-7.md`) が取り残され、whole-tree review が古い送付先を案内していた。(b) weekly reminder の閾値 7 日が **code default (30) / hooks-config.toml / ADR-070 / ADR-059 の 4 箇所**に分散し、うち 3 箇所が「暫定措置・再評価予定」という古い前提のままで、週次レビューの architecture facet がこれを finding として誤検出した (実際の drift は指摘と逆向きだった)。 + +## fallback を持つ設定値のテストは実際の解決経路を通す + +config が未指定のときに code default へ解決される (`config.foo.unwrap_or(DEFAULT)` 等) 設定値のテストは、**定数を直接ヘルパへ渡すのではなく、config が未指定の状態から解決関数を呼ぶ実経路**で書く。 + +1. **定数値の assert だけでは resolver の退行を検知できない** — `assert_eq!(DEFAULT, 7)` は定数を守るだけで、解決側が `unwrap_or(30)` に書き換わっても緑のまま通る。「config 行が無い環境でも既定で動く」という主張は、その経路を通して初めて検証される。 +2. **境界は解決経路の戻り値で固定する** — 閾値なら「境界の手前で発火しない / 境界で発火する」を、resolver を呼ぶ公開関数の戻り値 (`None` / `Some`) で assert する。 +3. **テストが効くことを変異で確かめる** — 既定値を別の値へ一時的に書き換えてテストが FAIL することを実測してから、元に戻す。テストが実際に何かを守っているかは、推論ではなく観測で確認する。 + +**由来** (2026-08-13 PR #396、CodeRabbit 指摘 + post-merge feedback): weekly reminder の既定値テストが定数を直接 `weekly_review_staleness_hits` へ渡しており、`reminder_threshold_days: None` から既定へ解決される経路を通っていなかった。実経路 (persisted `last_run_at` 経由) へ変更したところ、**テスト自身が `.claude` ディレクトリ未作成で落ちる隠れた前提**まで副次的に露見した。定数直渡しでは検出できなかった穴である。 + +## 複合タスクの仕様には各項目の処置と除外根拠を書く + +複数行 / 複数ファイル / 複数バッチにまたがるタスクを `docs/todo*.md` に書くときは、**対象として挙げた全項目について処置 (実施 / 除外) と、除外する場合の根拠**を仕様側に明記する。 + +1. **仕様側の対象数と実装側の対象数がずれる** — 「5 行が対象」と書いたタスクの実装が 4 行だけを扱っていても、根拠が書かれていなければレビューでは「意図的な絞り込み」と「見落とし」を区別できない。 +2. **除外は消さずに残す** — 除外した項目を仕様から削ると、次に読む人が「なぜこれは対象外なのか」を再調査することになる。 + +**由来** (2026-08-13 PR #395、PR diff + pre-push simplicity の 2 ソースが独立指摘): 週次レビュー採用の WR-2026-08-13-T02 が順位 203/216/228/239/240 の 5 行を降格対象と記述する一方、実装タスク T01 は Batch 1 の 4 行のみを扱い、**Batch 2 にある順位 216 の処置が仕様にも実装にも現れない**状態だった。 diff --git a/docs/todo-summary2.md b/docs/todo-summary2.md index de4583c5..72c8e4e7 100644 --- a/docs/todo-summary2.md +++ b/docs/todo-summary2.md @@ -181,6 +181,9 @@ | 441 | 🔧 Tier 2 | **cli-docs-lint に「詳細エントリ ⇄ 台帳行」の 1:1 対応検査を追加** | todo22.md | S-M | なし (2026-08-12 起票。todo14.md の孤児 4 件が 3 週間未検出だった lint 死角。本文順位番号 lint = 順位 334 と実装共有の可能性) | | 442 | 🔧 Tier 2 | **security facet に「新規 fail-closed 検査の抜けを敵対的に探す」観点を追加** | todo22.md | S | なし (2026-08-12 起票。ADR-056 確定判定の二重 miss 分析で最も再現性の高い失敗パターン = PR #313 Critical 3 件) | | 443 | 💎 Tier 3 | **fix 検証縮小 × re-gate 全 group 再実行の flaky 当たり面の縮小検討** | todo22.md | S-M | なし (2026-08-12 起票。ADR-058 確定判定で唯一の changed_block が flaky 誤 block と判明。negative result の永続化も正規の出口) | +| 444 | 🚀 Tier 1 | **orphan reaper が success report 検出時に meta.json を running のまま残す (feedback ループ恒久停止)** | todo22.md | S | なし (2026-08-13 起票。PR #396 マージで実発生。順位 398 の guard 変更で stale meta が初めてブロック要因化) | +| 445 | 🔧 Tier 2 | **todo preamble と facet routing 記述の整合を lint で機械検証** | todo22.md | S | なし (2026-08-13 起票。PR #395 feedback 採用。dev-conventions の暫定 convention を置換する) | +| 446 | 🚀 Tier 1 | **post-merge-feedback の transcript 抽出が並列 jj workspace のセッションを取りこぼす** | todo22.md | S | なし (2026-08-13 起票。PR #395 feedback 採用。ADR-030 の分析入力が無言欠落、まず切り分け) | **戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で計測基盤 (gate telemetry / weekly-review 保存) + rate-limit + convergence cost 削減を進める → Tier 3 でドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。(2026-08-12 更新: 旧記述の ADR-032 は ADR-057 置換で欠番) diff --git a/docs/todo22.md b/docs/todo22.md index fea17e90..f3fa2bb8 100644 --- a/docs/todo22.md +++ b/docs/todo22.md @@ -633,3 +633,128 @@ #### 完了基準 - 対処案の採否が根拠つきで決まり、採用案が実装または明文化されていること。 + +--- + +### 順位 444: orphan reaper が success report 検出時に meta.json を running のまま残す + +> **動機**: post-merge-feedback の run が**レポートを書いた後・meta.json を終端状態へ更新する前に死ぬ**と、その run は永久に「進行中」として残り、**以後すべての post-merge-feedback をブロックする**。 +> +> 2026-08-13 に PR #396 のマージで実発生した。ブロック元は #281 (2026-07-16 起動) と #374 (2026-08-09 起動) の 2 run で、どちらも `.claude/feedback-reports/{281,374}.md` の実体 (5000 B / 7471 B、mtime は起動の 15〜40 分後) を持ちながら `meta.json` が `status: "running"` / `currentStep: "analyze"` のままだった。 +> +> **発現は 2026-08-13 が初回で、それ以前はブロックしていない** (起票時に「#281 が 28 日間ブロックしていた」と書いたのは誤りで訂正済み)。旧 guard は「直前の起動から 1500 秒以内なら進行中」という時間窓判定で、stale meta を見ていなかったため無害だった。[PR #388](https://github.com/aloekun/claude-code-hook-test/pull/388) (順位 398、2026-08-11 land) が判定根拠を `meta.json` の `status` へ移したことで、既存の stale meta が初めて恒久ブロック要因に変わった。**さらに発現は exe のデプロイまで遅延した** — 同日 05:00Z の PR #395 の feedback は旧 exe で成功しており、`.claude/cli-merge-pipeline.exe` が #388 込みで再デプロイされた 08:00Z 以降、09:35Z の PR #396 で初めてブロックした。 +> +> **原因**: [reaper.rs](../src/hooks-session-start/src/reaper.rs) の `reap_orphans` は +> +> ```rust +> if marker.exists() || success_report.exists() { continue; } +> ``` +> +> で success report がある run を skip する。失敗マーカーを書かない判断自体は正しい (実際に成功しているため) が、**`mark_meta_failed` も呼ばれないため meta が `running` のまま残る**。一方 `cli-merge-pipeline` の進行中ガードは meta の `status` だけを見るため、この run を永久に「進行中」と解釈する。**2 コンポーネントの判定基準が不一致**で、reaper 側が「成功として整合させる」経路を持たないことが穴。 +> +> **参照**: [reaper.rs](../src/hooks-session-start/src/reaper.rs) (`reap_orphans` の skip 条件)、[run_registry.rs](../src/cli-merge-pipeline/src/feedback/run_registry.rs) (進行中判定)、[ADR-030](adr/adr-030-deterministic-post-merge-feedback.md)。順位 442 群と同じ「meta.json の status で進行中を判定する」ロジックの共有課題 (順位 428 系) とも隣接する。 +> +> **実行優先度**: 🚀 Tier 1 — Severity **High** (feedback ループが無言で恒久停止する。しかも停止に気づく手段が「マージ時の WARN」しかない) / Frequency Medium (run の異常終了は実際に 2 回発生) / Effort S / Adoption Risk None。 + +#### 設計決定 + +**2 層で直す。** reaper 側 (層 1) だけでも今回の事象は解けるが、それは「SessionStart が 1 回走る」ことに依存した回復であり、guard 自身は依然として stale meta 1 件で恒久停止する。guard 側 (層 2) を対称化しておくと、reaper が走る前でもブロックしない。 + +##### 層 1: reaper に reconcile 経路を追加する + +`reap_orphans` に「success report があるが meta が非終端」の**整合 (reconcile) 経路**を追加する。失敗マーカーは書かず、meta を `completed` 相当へ更新して進行中ガードを解放する。`endTime` はレポートの mtime から導出する (今回の手動復旧と同じ導出)。 + +- [ ] `reap_orphans` に reconcile 分岐を追加 (report あり + meta 非終端 → meta を終端化、marker は書かない) +- [ ] 回帰テスト: 既存 `reap_orphans_skips_when_success_report_exists_despite_stale_meta` は「skip する」ことを固定しているため、**この期待自体を「reconcile する」へ改める**必要がある (テスト名も含めて更新) +- [ ] 進行中ガード側 (`cli-merge-pipeline`) から見て、reconcile 後の run がブロック要因にならないことを確認する + +##### 層 2: guard の fail 方向を入力クラス間で対称化する + +[markers.rs](../src/cli-merge-pipeline/src/feedback/markers.rs) の `check_concurrent_run_guard` は、**隣接する 2 つの入力クラスで fail 方向が逆になっている**: + +| 入力 | 現在の扱い | 帰結 | +|---|---|---| +| meta.json がパース不能 | 進行中とみなさない (**fail-open**) | 意図的。doc に「壊れた meta.json 1 つで後続の feedback が永久に起動できなくなる方が害が大きい」と明記 | +| meta.json は読めるが `status: "running"` のまま古い | 進行中とみなす (**fail-closed**) | **恒久ブロック**。上の原則が想定していたのと同じ害が、評価されていない隣のクラスで起きる | + +順位 398 の doc が自ら述べた「1 件で永久に起動できなくなる方が害が大きい」という原則を、後者にも適用する。 + +**ただし「経過時間だけ」で fail-open にしてはならない。** 素朴に `startTime` から一定時間で切ると、**正常に長く走っている run を stale と誤判定**し、guard が同時実行を許して `context.json` 上書き防止という本来の目的を失う。しかも本 entry 自身が反例を持っている — 復旧した 2 run のレポート生成は**起動の 15〜40 分後**であり、reaper の `ORPHAN_THRESHOLD_SECS` (1500 秒 = 25 分) を共有すると **#374 の 40 分の run は「stale」と誤判定される**。閾値の共有は一見自然だが、reaper の閾値は「orphan と見なしてよい古さ」であって「post-merge-feedback の正常な実行時間の上限」ではなく、**目的が違う値を使い回してはならない**。 + +したがって経過時間は単独の判定根拠にせず、**生存/進捗の陽性シグナルと併用**する。候補: (a) run の `currentStep` / `currentIteration` が前回観測から進んでいるか、(b) `logs/` の最終更新、(c) プロセスの生存確認、(d) takt 側の heartbeat。いずれも「まだ動いている証拠」を積極的に取る形で、[ADR-064](adr/adr-064-monitor-success-positive-evidence.md) の陽性証拠要求と同じ姿勢を取る。 + +- [ ] 実測: post-merge-feedback の正常な実行時間分布を `.takt/runs/*/meta.json` の start/end から集計し、閾値を推測でなく実測から決める (本 entry の 15〜40 分は 2 サンプルにすぎない) +- [ ] `check_concurrent_run_guard` に「経過時間 + 生存/進捗シグナル」の複合判定を追加する (**経過時間だけの除外は入れない**) +- [ ] 回帰テスト: (i) 古く進捗も無い `running` はブロックしない、(ii) **閾値を超えていても進捗がある `running` はブロックする** (正常な長時間 run の誤除外防止)、(iii) 新しい `running` は従来どおりブロックする +- [ ] reaper の `ORPHAN_THRESHOLD_SECS` を**共有しない**判断を doc に残す (目的の違う閾値の使い回しを後から復活させないため) + +#### 完了基準 + +- success report を持つ非終端 run が SessionStart の reaper 通過後に終端状態へ整合され、`pnpm merge-pr` の進行中ガードを塞がないこと (層 1)。 +- **reaper が一度も走っていない状態でも**、閾値を超えた stale `running` が `pnpm merge-pr` をブロックしないこと (層 2)。 +- 閾値内の `running` は従来どおりブロックすること (guard の本来目的の非退行)。 +- 上記が回帰テストで固定され、`cargo test --workspace` が green であること。 + +--- + +### 順位 445: todo preamble と facet routing 記述の整合を lint で機械検証する + +> **動機**: `docs/todo.md` preamble が列挙する todo ファイル群 (新規追加先 / 編集専用 / 列挙範囲) と、それを参照する `.takt/facets/instructions/review-todo-whole.md` の routing 記述が**独立に手で維持されており、片方だけ古くなる**。 +> +> 2026-08-13 の PR #395 で実際に発生した: preamble の新規追加先が更新される一方、facet 側には `todo6.md` / `todo2-7.md` という**旧世代の固定値**が残り、whole-tree review が古い送付先を案内していた。`cli-docs-lint` は preamble の数詞は見るが**列挙範囲と実ファイル群の集合一致は検証しない**ため、この class は機械層に穴がある。weekly-review が 50KB 超過のたびに todo ファイルを増やす構造上、再発は継続的に起こる。 +> +> **参照**: [review-todo-whole.md](../.takt/facets/instructions/review-todo-whole.md) (routing 記述)、[docs/todo.md](todo.md) preamble、`src/cli-docs-lint/`、[ADR-007](adr/adr-007-custom-linter-layer-boundary.md) (正規表現層/AST 層の線引き)、[dev-conventions.md](dev-conventions.md) § 同一事実が複数箇所に分散する場合の変更手順 (本タスクが入るまでの暫定 convention)。 +> +> **実行優先度**: 🔧 Tier 2 — Severity Medium (誤誘導であり実行時破壊ではない) / Frequency **Medium** (todo ファイルは継続的に増える) / Effort S / Adoption Risk None。 + +#### 設計決定 + +`cli-docs-lint` に検査を追加する (custom lint rule ではなく docs-lint 側。preamble 解析は既に同 exe が持っているため)。 + +**集合の作り方を先に固定する。** ここを曖昧にすると誤検出か検査漏れのどちらかが必ず出る: + +- **対象は番号付きの詳細ファイルのみ** — `docs/todo*.md` の素の glob は `docs/todo-summary.md` / `docs/todo-summary2.md` も拾う。これらは順位 table であって詳細エントリの追加先ではないので、`todo<数字>.md` に限定する (`todo.md` 本体の扱いも明示的に決める)。 +- **範囲表記は展開してから比較する** — preamble と facet instruction はどちらも `todo3.md 〜 todo23.md` / `todo3-23.md` のような範囲表記を使う。文字列のまま集合比較すると常に不一致になるため、範囲を展開して要素の集合へ落とす。 +- **数詞と列挙範囲は別の検査** — 既存 `cli-docs-lint` は数詞 (「24 つ」) を見ているが、列挙範囲が実ファイル集合と一致するかは見ていない。本タスクで足すのは後者。 + +- [ ] 集合抽出規則を実装する (番号付き詳細ファイルのみ / 範囲表記の展開) +- [ ] preamble の列挙集合と `docs/todo<数字>.md` の実ファイル集合を比較する +- [ ] facet instruction 側の routing 記述に含まれる `todoN.md` 参照を抽出し、preamble の集合と矛盾しないか検査する +- [ ] fixture テスト (good / bad) を追加する。**bad 側に「summary ファイルを誤って含む」「範囲表記が未展開」の 2 ケースを必ず入れる** (本タスクの取りこぼし要因そのもの) +- [ ] 本タスク land 後、[dev-conventions.md](dev-conventions.md) § 同一事実が複数箇所に分散する場合の変更手順 の routing 該当部分を撤去する (ADR-042 のルール vs 仕組み化) + +#### 完了基準 + +- preamble と実ファイル群、preamble と facet routing 記述の不一致が `pnpm lint:docs` で検出されること。 +- 検出が fixture テストで固定され、`cargo test --workspace` が green であること。 + +--- + +### 順位 446: post-merge-feedback の transcript 抽出が並列 jj workspace のセッションを取りこぼす + +> **動機**: `cli-merge-pipeline` の transcript 抽出は `cwd` から導出した**単一 project-id フォルダ** (`~/.claude/projects//`) しか見ない。ところが本リポジトリは [ADR-045](adr/adr-045-jj-workspace-parallel-sessions.md) の並列 workspace 運用をしており、`~/.claude/projects/` には `c--Users-owner-work-claude-code-hook-test` と `C--Users-owner-work-claude-code-hook-test-improve` という**別 project-id のフォルダが実在する**。secondary workspace で実装したセッションの transcript は、現在のロジックから構造的に不可視になる。 +> +> 2026-08-13 の PR #395 の post-merge feedback で `session-analysis` が `session_data_unavailable` で失敗し、期待範囲 14 時間に対し無関係な 2.5 分の記録しか得られなかった。**[ADR-030](adr/adr-030-deterministic-post-merge-feedback.md) の前提であるセッション知見の抽出が systematic に無力化されうる**。 +> +> なお「PR #395 が実際に secondary workspace で実装されたか」は未確認で、まず**原因の切り分け**が要る (project-id フォルダが複数実在することは事実確認済み)。同じ「ハーネスが自分の壊れに気づけない」層の問題として順位 444 と隣接する。 +> +> **参照**: [transcript.rs](../src/cli-merge-pipeline/src/feedback/transcript.rs) (`project_transcript_dir` / `cwd_to_project_id`)、[ADR-045](adr/adr-045-jj-workspace-parallel-sessions.md)、[ADR-030](adr/adr-030-deterministic-post-merge-feedback.md)。 +> +> **実行優先度**: 🚀 Tier 1 — Severity **High** (feedback の分析入力が無言で欠落する) / Frequency Medium / Effort S (切り分けは短時間) / Adoption Risk None。 + +#### 設計決定 + +まず切り分け、結果に応じて対処を決める (先に実装を決めない)。 + +**対処を決める際の制約: 候補の絞り込みは「広げる」ではなく「束縛する」方向で行う。** (a) の全 project-id 走査や (c) の時刻範囲だけの検索は、**無関係なセッションを分析入力に引き込む**。実際 `~/.claude/projects/` には本リポジトリ以外の project-id も同居しうるし、同じ時間帯に別作業のセッションが走っていることもある。誤った transcript で feedback を生成するのは、transcript が無いより悪い (もっともらしい誤った知見が台帳へ入る)。したがって候補には **repo root / workspace path / PR 番号 / bookmark / session metadata のいずれかの陽性一致**を必須条件として課す ([ADR-064](adr/adr-064-monitor-success-positive-evidence.md) の陽性証拠要求と同じ姿勢)。canonical 化 (b) だけでは secondary workspace 配下に保存された transcript を発見できない場合がある点にも注意する。 + +- [ ] `~/.claude/projects/` の project-id 群と jj workspace 一覧を突合し、どの workspace がどの project-id に対応するかを確定する +- [ ] PR #395 の実装セッションがどの project-id 配下にあったかを特定し、取りこぼしが実際に起きたのかを確認する (起きていなければ別原因として本タスクを再定義する) +- [ ] 対処案を上記の束縛制約のもとで決める (探索範囲を広げる案は、必ず陽性一致条件とセットにする) +- [ ] 回帰テスト: 複数 project-id フォルダを持つ fixture で対象セッションが選ばれること。**bad ケースとして「無関係な project-id に同一時刻範囲のセッションがある」fixture を必ず含める** (時刻だけで拾わないことの担保) + +#### 完了基準 + +- 並列 workspace で実装したセッションの transcript が post-merge-feedback から参照できること。**fixture テストだけでは完了としない** — 実際に並列 workspace + 複数 project-id を用意し、`cli-merge-pipeline` の実経路 (exe 実行) で対象 transcript が選ばれることを確認する ([dev-conventions.md](dev-conventions.md) § LLM を含む自動化経路は実走でしか検証できない / [ADR-067](adr/adr-067-phase-b-unattended-fix-push.md))。 +- 無関係な project-id のセッションが選ばれないことも同じ実経路で確認すること。 +- 取りこぼしが再現しない場合は、**実行手順と観測結果を negative result として永続化**して閉じる ([dev-conventions.md](dev-conventions.md) § spike / 実験タスクの見送り (negative result) 永続化 convention)。