diff --git a/docs/todo-summary.md b/docs/todo-summary.md index af1d532f..6331155f 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -145,6 +145,12 @@ | 298 | 💎 Tier 3 | **token ベース ownership check の convention 化 (271.md T3-1 採用)** | todo13.md | S | なし (PID 再利用リスクという業界知見を dev-conventions.md に一般化して記載) | | 299 | 💎 Tier 3 | **revset で workspace 所有権を判定できない旨の convention 明記 (271.md T3-2 採用)** | todo13.md | XS | なし (`@` 厳密一致の設計判断という negative result を CLAUDE.md に明文化) | | 300 | 💎 Tier 3 | **Push pipeline 段階間依存性チェック項目の追加 (271.md T3-3 採用)** | todo13.md | S | なし (PR #271 の hidden coupling incident から得た教訓を CLAUDE.md / dev-conventions.md に恒久化) | +| 301 | 🚀 Tier 1 | **TOCTOU (remove+create_new) パターン検出 lint rule — exclusive lock 実装限定 (273.md T1-1 採用)** | todo13.md | S | なし (二重 Acquired バグの根本原因パターンを検出。cli-pr-monitor/lock.rs は設計判断済みのため scope 除外必須) | +| 302 | 💎 Tier 3 | **`takeover_stale_lock_skips_remove_when_snapshot_is_stale` パターンを deterministic concurrency test テンプレートとして記録 (273.md T2-3 採用)** | todo13.md | XS | なし (実スレッドレースより状態不一致を直接注入する決定論的テストパターンを次の並行処理系 PR 向けに記録) | +| 303 | 💎 Tier 3 | **Advisory lock (fail-open) の TOCTOU window 許容可否を明示コメントで残す設計チェックリスト (273.md T3-1 採用)** | todo13.md | XS | なし (cli-pr-monitor/lock.rs が既に実践している判断根拠明示の practice をチェックリスト化) | +| 304 | 💎 Tier 3 | **quality gate 実行中に発見したバグ修正が別 PR に混入した際の jj split + jj rebase 復旧パターンを記録 (273.md T3-3 採用)** | todo13.md | XS | なし (PR #272/#273 分離で実証済みの復旧手順、ADR-045 の並列 workspace リスクとは別種の単一 session 内混入事故。復旧は事後対応であり、分離後は混在した変更に対する gate 実行結果を無効化し各 PR で再実行する手順を含む) | +| 305 | 💎 Tier 3 | **Metrics violation の pre-existing 判定基準の明文化 (273.md T3-4 採用)** | todo13.md | XS | なし (file_size_check / file_length_gate 等 metrics 系 gate が複数稼働中で反復しうる override 正当性の判定基準を明文化) | +| 306 | 💎 Tier 3 | **quality gate isolation 機構を見送り、recovery による risk acceptance とした判断の記録 (negative result) (273.md T3-5 採用)** | todo13.md | S | なし (spike 見送り convention に従い、isolation 機構を却下し recovery コストの低さ (順位304) を理由に risk acceptance した根拠を記録。recovery は isolation の代替ではなく、予防機能の欠如という残存リスクと再検討条件を明記する) | **戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。 diff --git a/docs/todo13.md b/docs/todo13.md index 12405fbf..f9eb590c 100644 --- a/docs/todo13.md +++ b/docs/todo13.md @@ -1220,6 +1220,138 @@ --- +### TOCTOU (remove+create_new) パターン検出 lint rule — exclusive lock 実装限定 (273.md T1-1 採用) + +> **動機**: PR #273 の二重 Acquired バグ (`remove_file` 直前の状態再検証欠落) は data integrity violation の根本原因だった。`remove_file` の直前に安全性を示す justification コメントが無い exclusive lock 実装を検出する custom lint rule (rule⑩ `no-write-result-discard` と同型の comment-presence 検出) を追加する。 +> +> **重要な scope 限定**: `cli-pr-monitor/src/lock.rs` の `MonitorLock` は `std::fs::write` overwrite 方式 + 「stale takeover の race は benign」という設計判断をコメントで既に明示済みであり、本 rule の対象外とすべき (混同すると誤検出になる)。paths を `pipeline_lock.rs` 等の exclusive-lock 実装ファイルに限定して実装すること。 +> +> **既知の限界と過去の関連判断**: 271.md Tier 1 #1 (「Concurrent guard (Drop) の無条件リソース削除検出」regex 検出) は「regex では検証済み/未検証を区別できず ADR-007 の regex 層限界に抵触する」という理由で**既に却下済み**。本エントリの単純な comment-presence 検出も同じ限界 (justification コメントさえあれば実際の再検証コードが無くても通過してしまう) を抱える。CodeRabbit re-review (PR #274) 指摘によりこの限界が具体化したため、下記のとおり検出粒度を「コメント有無」から「再読込→比較→remove_file という 3 ステップの出現順序」の regex/pattern 検出へ強化する (AST 層への格上げは Effort M 相当となり本エントリの Effort S を超えるため、まずは pattern 検出の強化で対応し、それでも false negative が実運用で頻発する場合に AST 層格上げを再検討する)。 +> +> **参照**: `.claude/feedback-reports/273.md` Tier 1 #1、`.claude/feedback-reports/271.md` Tier 1 #1 (関連する過去の却下判断)、`src/lib-jj-helpers/src/pipeline_lock.rs` (今回の fix)、`.claude/custom-lint-rules.toml` +> +> **実行優先度**: 🚀 Tier 1 — Severity High / Effort S。 + +#### 作業計画 + +- [ ] `.claude/custom-lint-rules.toml` に「`remove_file` 呼び出し directly 手前の N 行以内に、読込 (`read_to_string` 等) → 比較 (`==`/`if let` 等) の出現順序があること」を要求する pattern 検出ルールを追加 (単純な comment-presence ではなく構造的な出現順序を見る、paths を exclusive-lock 実装限定) +- [ ] `cli-pr-monitor/src/lock.rs` を誤検出しないことを確認する negative fixture 追加 +- [ ] 「justification コメントはあるが再読込・比較コードが無い」ケースが lint により検出される (= コメントのみでは通過しない) ことを示す negative fixture を追加 +- [ ] lint 検出時に CODE REVIEW で「lock safety pattern verified」を人手確認する運用を `docs/dev-conventions.md` に明文化し、本 rule の false negative となりうるケース (カバレッジ限界) を rule 定義コメントに記録 +- [ ] 本エントリ削除 + todo-summary.md 行削除 + +#### 完了基準 + +- 「読込→比較→remove_file」という構造そのものを欠く新規 exclusive lock 実装が、lint rule (pattern 検出) により push 前に検出されること。 +- 「justification コメントのみで再検証コードを欠く」実装が、コメントの存在にかかわらず lint で検出される (= 通過しない) ことが negative fixture で証明されていること。 +- 上記 pattern 検出にも false negative となりうるケースが残るため、lint 検出時に CODE REVIEW で「lock safety pattern verified」であることを人手確認する運用が明文化されていること、かつ本 rule のカバレッジ限界が記録されていること。 + +--- + +### `takeover_stale_lock_skips_remove_when_snapshot_is_stale` パターンを deterministic concurrency test テンプレートとして記録 (273.md T2-3 採用) + +> **動機**: PR #273 で追加した決定論的 regression test (`stale_snapshot` を意図的に不一致にして takeover レースを注入的に再現するパターン) は、実スレッドタイミングに依存する flaky test (`concurrent_stale_takeover_only_one_wins`) より再現性が高い。次の並行処理系 PR で同型テストが必要になった際のテンプレートとして記録する。 +> +> **参照**: `.claude/feedback-reports/273.md` Tier 2 #3、`src/lib-jj-helpers/src/pipeline_lock.rs` の `takeover_stale_lock_skips_remove_when_snapshot_is_stale` +> +> **実行優先度**: 💎 Tier 3 — Effort XS。 + +#### 作業計画 + +- [ ] `docs/dev-conventions.md` に「並行処理の regression test は実スレッドレースより、内部関数を直接呼び状態不一致を注入する決定論的パターンを優先する」旨とコード例を追記 +- [ ] 本エントリ削除 + todo-summary.md 行削除 + +#### 完了基準 + +- 次の並行処理系バグ修正で、決定論的テストパターンが参照可能な形で存在すること。 + +--- + +### Advisory lock (fail-open) の TOCTOU window 許容可否を明示コメントで残す設計チェックリスト (273.md T3-1 採用) + +> **動機**: `cli-pr-monitor/src/lock.rs` の `MonitorLock` は「stale takeover の race は benign」という判断を既にコメントで明示済みだが、これは実践のみでチェックリスト化されていない。既に実践されている practice を明文化すれば、将来の advisory lock 実装での判断ミス (許容可否を検討せず TOCTOU を放置する、あるいは過剰に厳格化する) を構造的に防止できる。 +> +> **参照**: `.claude/feedback-reports/273.md` Tier 3 #1、`src/cli-pr-monitor/src/lock.rs`、`src/lib-jj-helpers/src/pipeline_lock.rs` (takeover_stale_lock の doc comment) +> +> **実行優先度**: 💎 Tier 3 — Effort XS。 + +#### 作業計画 + +- [ ] `docs/dev-conventions.md` に「advisory lock の TOCTOU window に触れる実装は、許容可否の判断根拠を doc comment に残す」チェックリストを追加 +- [ ] 本エントリ削除 + todo-summary.md 行削除 + +#### 完了基準 + +- advisory lock 実装時に参照できるチェックリストが存在すること。 + +--- + +### quality gate 実行中に発見したバグ修正が別 PR に混入した際の `jj split` + `jj rebase` 復旧パターンを記録 (273.md T3-3 採用) + +> **動機**: PR #272 (docs-only) の push 中に quality gate が実行した `cargo test --workspace` で PR #273 相当のバグを発見し、その場で修正した結果 docs コミットに混入した。`jj split` + `jj rebase` で低コストに復旧できた実務パターンを記録する。ADR-045 の並列 workspace リスクとは別種の事故 (単一 session 内の混入) であり、区別して記録する価値がある。 +> +> **参照**: `.claude/feedback-reports/273.md` Tier 3 #3、本セッションの復旧手順 (`jj split -m ... ` → `jj rebase -s -d ` → `jj rebase -s -d master`) +> +> **実行優先度**: 💎 Tier 3 — Effort XS。 + +#### 作業計画 + +- [ ] `docs/dev-conventions.md` に「push/merge パイプライン実行中に無関係なバグを発見・修正した場合、`jj split` で分離し、それぞれ独立した bookmark/PR にする」復旧手順を追記 +- [ ] `jj split`/`jj rebase` は**混入後の事後対応**であり、混在した変更に対して既に実行された quality gate / pre-push review の結果は汚染されている (予防はできていない) ため、分離後は当該結果を破棄し、分離後の各コミット/PR で quality gate / pre-push review を個別に再実行する手順を追記 +- [ ] 本エントリ削除 + todo-summary.md 行削除 + +#### 完了基準 + +- 同種の混入が今後発生した際に、参照できる復旧手順が存在すること。 +- 復旧手順に「混在した変更に対する gate 実行結果は無効であり、分離後に各 PR で個別に再実行する」ことが明記されていること (CodeRabbit 指摘: 復旧は予防の代替ではなく、汚染された gate 結果をそのまま信頼してはならない)。 + +--- + +### Metrics violation の pre-existing 判定基準の明文化 (273.md T3-4 採用) + +> **動機**: metrics 系 gate (`file_size_check` / `file_length_gate` 等) が複数稼働中の本リポジトリでは、violation が先行 PR/feature 由来の pre-existing なものか、今回の変更に起因するものかを判定して override する場面が繰り返し発生する。PR #273 では 4 件の violation が PR #271 由来の pre-existing として人手判断で正しく override されたが、判定基準 (対象 revset の選び方・feature 境界の見極め方) が曖昧なまま自動化すると誤判定リスクがある。判定基準の明文化は Tier 2 #5 (自動 exemption 機構) の検討の前提を整える。 +> +> **参照**: `.claude/feedback-reports/273.md` Tier 3 #4、Tier 2 #5、`docs/dev-conventions.md` +> +> **実行優先度**: 💎 Tier 3 — Effort XS。 + +#### 作業計画 + +- [ ] `docs/dev-conventions.md` に「metrics violation が pre-existing と判断する際の判定基準 (対象 revset の選び方、feature 境界の見極め方など)」チェックリストを追加 (基準時点/現時点の計測結果・差分、判定理由、判定者・判定日時、レビュー承認者を記録する audit trail 要件を含み、証跡が揃わない場合は override 不可とする) +- [ ] 本エントリ削除 + todo-summary.md 行削除 + +#### 完了基準 + +- metrics 系 gate の violation を pre-existing として override する際に、判断根拠として参照できる基準が存在すること。 +- 上記基準に加え、override 判定時に「基準時点と現時点の計測結果・差分」「pre-existing と判断した理由」「判定者・判定日時」「レビュー承認者」を PR/MR コメントまたは `docs/override-log.md` に記録し、これらの証跡が揃わない限り override できないチェックリストになっていること (同一メトリクスの反復 violation を将来 anomaly として検知できるようにするため)。 + +--- + +### quality gate isolation 機構を見送り、recovery による risk acceptance とした判断の記録 (negative result) (273.md T3-5 採用) + +> **動機**: PR #273 の post-merge-feedback は「quality gate 実行を commit group ごとに isolated working copy で行う構造的防止機構」(Tier 2 #4) を提案したが、Effort L・runner 複雑化という Adoption Risk に見合わず却下した。spike 見送り (negative result) 永続化 convention に従い、この却下判断を記録する。 +> +> **CodeRabbit 指摘 (PR #274) による訂正**: **recovery (`jj split`/`jj rebase` 復旧パターン) は isolation (予防) の代替にはならない。** isolation は「混入自体を未然に防ぐ」機構であり、recovery は「混入が起きたことを検知した後に事後対応する」機構であって、両者は異なるリスク層に属する。isolation を見送った真の判断は「recovery で同等の予防効果が得られる」ではなく、「混入は今後も起こりうるが、発生時の recovery コストが低いため、isolation 実装コスト (Effort L) をかけてまで予防する必要はないと risk acceptance した」という判断である。 +> +> **参照**: `.claude/feedback-reports/273.md` Tier 2 #4 (却下 recommendation)、Tier 3 #5、docs/dev-conventions.md § spike 見送り (negative result) 永続化 convention、`jj split`/`jj rebase` 復旧パターンを記録するタスク (本ファイル内) +> +> **実行優先度**: 💎 Tier 3 — Effort S。 + +#### 作業計画 + +- [ ] 関連 ADR (ADR-045 または新規 amendment) に、isolation 機構を見送り、recovery コストの低さを理由に risk acceptance した判断を negative result として記録する。「recovery が isolation の代替になる」という表現は用いない +- [ ] 記録には「isolation を見送ったことで残る予防機能の欠如 (混在した変更に対して quality gate / pre-push review が誤って green 判定を出しうる残存リスク)」を明記する +- [ ] 記録には再検討条件 (例: 同種の混入事故が反復する、isolation の実装コストが下がる、等) を明記する +- [ ] `docs/todo-summary.md` の本エントリ行の説明も「代替」ではなく「recovery コストの低さによる risk acceptance」と表現する +- [ ] 本エントリ削除 + todo-summary.md 行削除 + +#### 完了基準 + +- 将来の再検討時に、この見送り判断の根拠が参照可能であること。 +- 記録が「recovery は isolation の代替である」という誤解を招く表現になっておらず、予防機能の欠如という残存リスクと、再検討条件が明記されていること。 + +--- +