Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/todo-summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 への直接効果は小さい。

Expand Down
132 changes: 132 additions & 0 deletions docs/todo13.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 ... <file>` → `jj rebase -s <docs-commit> -d <docs-parent>` → `jj rebase -s <fix-commit> -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 する際に、判断根拠として参照できる基準が存在すること。
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- 上記基準に加え、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 の代替である」という誤解を招く表現になっておらず、予防機能の欠如という残存リスクと、再検討条件が明記されていること。

---




Expand Down
Loading