diff --git a/docs/local-llm-offload-analysis.md b/docs/local-llm-offload-analysis.md index 6f075c7..e526583 100644 --- a/docs/local-llm-offload-analysis.md +++ b/docs/local-llm-offload-analysis.md @@ -417,7 +417,7 @@ P-0 (config opt-in): PR #123, merged 2026-05-07 ✅ (本ファイル §10 govern P-1 (Bundle g-1): PR #125, merged 2026-05-07, findings: 0 (CR APPROVE no comments), classifier 未起動 (input data なし), 計測 N/A — dogfood 不発 P-2 (順位 47): PR #126, merged 2026-05-07, findings: 1 (Nitpick, CR review body 内 `
` block), agreement: 1/1 (100%, 私評価=human_review と一致), latency: 6.4s/件 (>5s 目標), fallback: 1/1 (normalized_issue length 100>80) - 既知 gap: check-ci-coderabbit が review body の `
` block 内 Nitpick を抽出しない (post-pr-monitor が classifier に渡せず、手動で synthetic finding 構築して classifier 実行) -P-3 (順位 7): PR #___, merged ___, findings: __, agreement: __/__, token Δ: __, latency: __s/件, fallback: __/__ +P-3 (順位 7): PR #127, merged 2026-05-08, findings: rate-limit blocked (CR が 27 min wait の rate-limit overlay で formal review 投稿不可)、classifier 未起動、計測 N/A — dogfood 不発 (但し CR rate-limit 経路が dogfood の阻害要因として観測された、§A-2 §6 注意事項 #1 を実証) P-4 (順位 76+77): PR #___, merged ___, findings: __, agreement: __/__, token Δ: __, latency: __s/件, fallback: __/__ P-5 (Bundle f-1): PR #___, merged ___, findings: __, agreement: __/__, token Δ: __, latency: __s/件, fallback: __/__ diff --git a/docs/todo.md b/docs/todo.md index 60ed33c..706be61 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -65,8 +65,6 @@ | 67 | 💎 Tier 3 | **ADR-030 に abrupt 終了時の振る舞いを spec として明記 (PR #109 T3-2 採用) ★ Bundle c** | todo5.md | XS | 順位 63 / 64 と同 PR (実装と仕様の整合性確保、L1 in-process Drop guard + L2 out-of-process reaper の責務分離 + SLA 化) | | 68 | 🔧 Tier 2 | **`no-ephemeral-todo-reference` self-exclusion invariant 単体テスト追加 (PR #110 T2-1 採用) ★ Bundle d** | todo5.md | S | なし (PR #110 直接対策、placeholder N 戦略の machine-enforceable 保護、TP/FP/Edge 3 軸テスト) | | 69 | 💎 Tier 3 | **`no-ephemeral-todo-reference` の `yaml`/`yml` extensions 追加理由をコメントで明記 (PR #110 T3-1 採用) ★ Bundle d** | todo5.md | XS | なし (rule⑥ コメント欄に 1-2 行追記、設計 doc と実装の経緯保存、git blame 不要化) | -| 76 | 🔧 Tier 2 | **config sanitize → poll arithmetic の cross-module overflow 統合テスト (PR #115 T2-1 採用) ★ Bb-3 follow-up** | todo5.md | M | なし (PR #115 で `review_recheck_sanitize_keeps_i64_max_boundary` が unit test isolation のため cross-module overflow を見逃した実証ベース、`MAX_SAFE_WAIT_SECS` / `0` / `u64::MAX inject` の全 code path で `poll.rs` の `now_unix + wait_secs as i64` が overflow しないことを統合 test で machine-enforce) | -| 77 | 🔧 Tier 2 | **Unix timestamp baseline での境界値テスト matrix (PR #115 T2-2 採用) ★ Bb-3 follow-up** | todo5.md | S | なし (`MAX_SAFE_WAIT_SECS = 1年` の根拠が現在の timestamp ~1.7e9 に依存する time-dependent。`now + 0` / `now + MAX_SAFE_WAIT_SECS` / `now + MAX_SAFE_WAIT_SECS + 1` の境界値で i64 overflow safety を自動検証、user-editable config boundary として今後の変更にも追随) | | 78 | 💎 Tier 3 | **ADR-038 (Rust timestamp arithmetic safety) + CLAUDE.md security 拡充 (PR #115 T3-1 採用) ★ Bb-3 follow-up** | todo5.md | S | なし (config が user-editable system boundary のとき `sanitize()` 値域検証を必須化し dependent arithmetic に `// SAFETY: により上限保証` コメントを要求するパターンを ADR + CLAUDE.md に codify、Rust 固有の checked_add + MAX_SAFE capping + time-dependent test の 3 層を明文化) | | 79 | 💎 Tier 3 | **`docs-governance.md` § Retirement Workflow に「残タスクの lifecycle 整合」要件明記 (PR #117 T3-1 採用)** | todo5.md | XS | なし (PR #117 で順位 15 を Bb-3 で吸収済として削除した際、現 Step 2「残タスクを priority table に登録」が priority table から除外するケース = 完了/deprioritize/defer を未定義だった実証。除外時の commit/PR で 3 値のいずれかを明示する要件を追加して将来の同型 ambiguity を構造的に防ぐ) | | 80 | 🚀 Tier 1 | **cli-pr-monitor: rate-limit auto-retry wakeup 予約ロジックの整理 (PR #120 T1-1 採用) ★ Bundle f** | todo5.md | M | なし (PR #120 dogfood で `auto_retry_enabled=true` でも wakeup park が予約されず exit する事象を観測、ADR-018 設計の実装落ち) | diff --git a/docs/todo5.md b/docs/todo5.md index 73aca62..380fc83 100644 --- a/docs/todo5.md +++ b/docs/todo5.md @@ -658,85 +658,6 @@ --- -### config sanitize → poll arithmetic の cross-module overflow 統合テスト (PR #115 T2-1 採用) ★ Bb-3 follow-up - -> **動機**: PR #115 の sanitize() 実装後、CR から Major #2 として「`review_recheck_sanitize_keeps_i64_max_boundary` テストが `i64::MAX as u64` を valid として通すが、poll.rs の `now_unix + wait_secs as i64` で確実に overflow する」という **unit test isolation 起因の自己矛盾**を指摘された。即修正で `MAX_SAFE_WAIT_SECS = 1 年` に上限を変更したが、原因は **「config layer の単体テストが downstream arithmetic の overflow safety を確認していなかった」**こと。同類の cross-module integrity 問題は今後も sanitize() に新フィールド追加・既存値の境界変更で再発しうる。 -> -> **本タスクの位置づけ**: Bb-3 完成 (PR #115 マージ済) 後の follow-up。post-merge-feedback Tier 2 #1 採用。 -> -> **参照**: PR #115 CR Major #2 (id 3192783887)、`.claude/feedback-reports/115.md` Tier 2 #1、`src/cli-pr-monitor/src/config.rs` (sanitize)、`src/cli-pr-monitor/src/stages/poll.rs` line 662 / 734 (cast points) -> -> **実行優先度**: 🔧 **Tier 2** — Effort M。順位 77 (境界値テスト matrix) と同一 PR で land すると DRY (両方とも `wait_secs` boundary を扱う)。 - -#### 設計決定 (案) - -- **テストの配置**: `src/cli-pr-monitor/src/stages/poll.rs` の `#[cfg(test)] mod tests` 内に integration test 風のセクションを追加 (config + poll を貫通させる) -- **テストの形**: 以下 3 経路を verify - 1. `MAX_SAFE_WAIT_SECS` を sanitize() 通過後、`finalize_initial_review_park` / `schedule_next_review_recheck_park` 経由で書き込まれる `state.next_wakeup_at_unix` が `i64::MAX` 以下 (= overflow しない) - 2. `0` を sanitize() で default に置換後、書き込まれる `state.next_wakeup_at_unix` が `now_unix + 300` (default fallback 動作) - 3. `u64::MAX` を sanitize() で default に置換後、書き込まれる `state.next_wakeup_at_unix` が `i64::MAX` 以下 (= overflow しない) -- **invariant assertion**: `state.next_wakeup_at_unix > now_unix && state.next_wakeup_at_unix < i64::MAX` を全 path で assert -- **test fault injection**: 既存の `PR_MONITOR_STATE_FILE_OVERRIDE` env var + `env_override_lock` Mutex を再利用 (T2-2 で確立済の pattern) - -#### 作業計画 - -- [ ] `poll.rs` test module に `cross_module_overflow_safety_<シナリオ>` テスト 3 件追加 -- [ ] 全 path で `state.next_wakeup_at_unix` の値域 invariant を assert -- [ ] `sanitize() を経由しない直接 inject 経路` (= テスト用 helper で異常値を直接書く) も対象に追加し、sanitize layer がない場合の overflow を **negative test** で確認 (sanitize の保護が機能していることの裏付け) -- [ ] cargo test 全 pass を確認、CI 統合 -- [ ] 派生プロジェクト deploy には影響なし (test only) -- [ ] 本 todo5.md エントリを削除 - -#### 完了基準 - -- 統合テスト 3-4 件追加 (positive + negative) -- `now_unix + sanitize 後の wait_secs` が i64 overflow しないことを machine-enforce -- 将来 `MAX_SAFE_WAIT_SECS` を変更したり sanitize() に新フィールド追加した際、テストが落ちて invariant 違反を検知 - -#### 詰まっている箇所 - -- `now_unix` の取得を test 環境で固定する仕組み (現実装は `SystemTime::now()` で test 不安定要因)。最低限「`SystemTime::now() + sanitize 後の値 < i64::MAX`」が常に成立することを assert する形で済ます (実環境の `now_unix` は ~1.78e9 << i64::MAX なので、sanitize 後の値が `MAX_SAFE_WAIT_SECS = 1 年 = 3.15e7` 以下なら確実に safe) - ---- - -### Unix timestamp baseline での境界値テスト matrix (PR #115 T2-2 採用) ★ Bb-3 follow-up - -> **動機**: `MAX_SAFE_WAIT_SECS = 365 * 24 * 60 * 60` の根拠 (i64 overflow safety) は **現在の Unix timestamp が ~1.78e9 (2026 年)** に依存する time-dependent な前提。例えば 2100 年の時点では `now_unix` が ~4.1e9 に達し、`now_unix + MAX_SAFE_WAIT_SECS` が i64::MAX に対する safety margin が縮む (= 計算上は安全だが、根拠の説明性が弱まる)。境界値を明示的にテストすることで `MAX_SAFE_WAIT_SECS` の妥当性を future-proof に保証。 -> -> **本タスクの位置づけ**: 順位 76 と同一 PR で land 推奨 (両方とも boundary を扱う)。post-merge-feedback Tier 2 #2 採用。 -> -> **参照**: PR #115 `MAX_SAFE_WAIT_SECS` 定数 (config.rs)、`.claude/feedback-reports/115.md` Tier 2 #2 -> -> **実行優先度**: 🔧 **Tier 2** — Effort S。順位 76 と統合 PR で land。 - -#### 設計決定 (案) - -- **テストの形**: `MAX_SAFE_WAIT_SECS` の境界値 matrix を `config.rs` test module に追加 - - `now_2026 = 1_800_000_000_i64` (~ 2026 年) baseline - - `now_2100 = 4_100_000_000_i64` (~ 2100 年) baseline (future-proof check) - - 各 baseline で `now + MAX_SAFE_WAIT_SECS` / `now + MAX_SAFE_WAIT_SECS + 1` の i64 overflow safety を `checked_add` で assert -- **既存 `review_recheck_sanitize_max_safe_boundary` テストを拡張**: 現実装は `now_unix_2026` 単体だが、`now_2100` も追加して未来の Unix timestamp でも safe であることを machine-enforce -- **コメント補強**: `MAX_SAFE_WAIT_SECS` 定数の doc comment に「now_2100 + 1 年 < i64::MAX で safety margin あり」を明記 - -#### 作業計画 - -- [ ] `config.rs` test module の `review_recheck_sanitize_max_safe_boundary` を拡張 (now_2026 + now_2100 の 2 baseline) -- [ ] `MAX_SAFE_WAIT_SECS` doc comment に future-proof の根拠 (year 2100 でも safe) を追記 -- [ ] cargo test 全 pass を確認 -- [ ] 順位 76 と同 PR で land -- [ ] 本 todo5.md エントリを削除 - -#### 完了基準 - -- `now_2100 + MAX_SAFE_WAIT_SECS` が `checked_add` で `Some(_)` を返すこと (= overflow しない) を assert -- `MAX_SAFE_WAIT_SECS` 定数 doc に future-proof 根拠が記述される - -#### 詰まっている箇所 - -なし (Effort S、test 拡張 + doc コメント追加のみ) - ---- - ### ADR-038 (Rust timestamp arithmetic safety) + CLAUDE.md security 拡充 (PR #115 T3-1 採用) ★ Bb-3 follow-up > **動機**: PR #115 で「config が user-editable system boundary のとき、sanitize() で値域検証 + 下流 arithmetic で安全範囲保証」というパターンが実証された (CR Major #1 + #2 が両方とも同型の「config 値→arithmetic 入力」cross-layer integrity 問題)。同型の bug class は今後も Rust + config 駆動の component で発生しうるため、組織的 learning として codify。 diff --git a/src/cli-pr-monitor/src/config.rs b/src/cli-pr-monitor/src/config.rs index bb055b2..15820c0 100644 --- a/src/cli-pr-monitor/src/config.rs +++ b/src/cli-pr-monitor/src/config.rs @@ -167,6 +167,11 @@ impl Default for ReviewRecheckConfig { /// 加算で確実に算術 overflow し、release build では負の wakeup_at にラップする。 /// 1 年 = 3.15e7 << i64::MAX = 9.22e18 で `now_unix + 1年` は overflow しない。 /// CronCreate の auto-expire は 7 日のため、1 年は user 編集の上限として十分な余裕を持つ。 +/// +/// Future-proof 根拠 (順位 77): year 2100 baseline (`now_unix ~ 4.1e9`) でも +/// `4.1e9 + 3.15e7` = 4.13e9 << i64::MAX (9.22e18) で safety margin が +/// 約 9 桁 (2.2e9 倍) ある。`review_recheck_sanitize_max_safe_boundary` test +/// で 2026 + 2100 baseline 両方を machine-enforce。 const MAX_SAFE_WAIT_SECS: u64 = 365 * 24 * 60 * 60; impl ReviewRecheckConfig { @@ -578,5 +583,122 @@ timeout_secs = 60 safe_sum.is_some(), "sanitize 後の値は now_unix + wait で overflow しない (CR Major #2 invariant)" ); + + let now_unix_2100: i64 = 4_100_000_000; + let safe_sum_2100 = + now_unix_2100.checked_add(cfg_at_limit.initial_review_wait_secs as i64); + assert!( + safe_sum_2100.is_some(), + "year 2100 baseline でも overflow しない (順位 77: future-proof)" + ); + assert!( + safe_sum_2100.unwrap() < i64::MAX, + "year 2100 baseline で safety margin が残る" + ); + } + + #[test] + fn cross_module_overflow_safety_at_max_boundary() { + let cfg = ReviewRecheckConfig { + initial_review_wait_secs: MAX_SAFE_WAIT_SECS, + review_recheck_wait_secs: MAX_SAFE_WAIT_SECS, + max_review_rechecks: 1, + } + .sanitize(); + + let now_unix: i64 = 1_800_000_000; + let initial_park_sum = now_unix.checked_add(cfg.initial_review_wait_secs as i64); + let recheck_park_sum = now_unix.checked_add(cfg.review_recheck_wait_secs as i64); + + assert!( + initial_park_sum.is_some(), + "finalize_initial_review_park の next_wakeup_at_unix 加算が overflow しない" + ); + assert!( + recheck_park_sum.is_some(), + "schedule_next_review_recheck_park の next_wakeup_at_unix 加算が overflow しない" + ); + assert!( + initial_park_sum.unwrap() > now_unix, + "next_wakeup_at_unix が past に巻き戻らない" + ); + assert!( + recheck_park_sum.unwrap() > now_unix, + "next_wakeup_at_unix が past に巻き戻らない" + ); + } + + #[test] + fn cross_module_overflow_safety_with_zero_input_uses_default() { + let cfg = ReviewRecheckConfig { + initial_review_wait_secs: 0, + review_recheck_wait_secs: 0, + max_review_rechecks: 1, + } + .sanitize(); + + assert_eq!( + cfg.initial_review_wait_secs, 300, + "0 入力は sanitize で default 300s に置換" + ); + assert_eq!(cfg.review_recheck_wait_secs, 300); + + let now_unix: i64 = 1_800_000_000; + let park_sum = now_unix + .checked_add(cfg.initial_review_wait_secs as i64) + .unwrap(); + assert_eq!(park_sum, now_unix + 300); + } + + #[test] + fn cross_module_overflow_safety_with_u64_max_input_uses_default() { + let cfg = ReviewRecheckConfig { + initial_review_wait_secs: u64::MAX, + review_recheck_wait_secs: u64::MAX, + max_review_rechecks: 1, + } + .sanitize(); + + assert_eq!( + cfg.initial_review_wait_secs, 300, + "u64::MAX 入力は sanitize で default 300s に置換" + ); + assert_eq!(cfg.review_recheck_wait_secs, 300); + + let now_unix: i64 = 1_800_000_000; + let park_sum = now_unix + .checked_add(cfg.initial_review_wait_secs as i64) + .unwrap(); + assert!(park_sum < i64::MAX); + } + + #[test] + fn cross_module_overflow_safety_negative_test_large_unsanitized_value_overflows() { + let unsanitized_wait: u64 = i64::MAX as u64; + let now_unix: i64 = 1_800_000_000; + let unsafe_sum = now_unix.checked_add(unsanitized_wait as i64); + + assert!( + unsafe_sum.is_none(), + "sanitize なしで i64::MAX 近傍の wait を直接 cast すると positive overflow する (sanitize の必要性の裏付け、PR #115 CR Major #2 の再現)" + ); + } + + #[test] + fn cross_module_overflow_safety_negative_test_u64_max_wraps_to_minus_one() { + let unsanitized_wait: u64 = u64::MAX; + let now_unix: i64 = 1_800_000_000; + let wrapped = unsanitized_wait as i64; + + assert_eq!( + wrapped, -1, + "u64::MAX as i64 は -1 (two's complement)、checked_add は overflow しないが過去時刻になる" + ); + + let result = now_unix.checked_add(wrapped).unwrap(); + assert!( + result < now_unix, + "u64::MAX を経由した wakeup_at は過去にずれる (= silent corruption、sanitize で防止)" + ); } }