diff --git a/docs/todo-summary.md b/docs/todo-summary.md index fe9844c8..5340a5e4 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -86,7 +86,6 @@ | 205 | 💎 Tier 3 | **`~/.claude/rules/common/git-workflow.md` に jj auto-snapshot onboarding rule 追記 (PR #201 post-merge-feedback T3-4 採用)** | todo10.md | XS | なし (PR #201 で prior session の docs commit 199-202 と本セッションの impl 146 が auto-snapshot で混入し bundle 化に収束した実観測、jj Operations section に「Auto-snapshot の理解と logical separation」sub-section 追加、`jj new -m` を **作業開始時** に実行する正しいフロー明文化、派生プロジェクトへ global 経由で自動波及、`feedback_global_config_backup` 適用必須) | | 206 | 💎 Tier 3 | **`~/.claude/rules/common/development-workflow.md` § 1. Plan First に「todo*.md 分割時の todo-summary.md 同一 commit 更新」checklist 追加 (PR #204 post-merge-feedback T3-1 採用)** | todo10.md | S | なし (PR #133 + #153 + #204 の 3 PR 連続観測で multi-file artifact split 時の永続 index 更新漏れが Frequency Medium 閾値到達、3 step checklist (分割エントリ列挙 / sed 一括 file 列更新 / 同一 commit) を § 1. Plan First の Codification 重複確認 step 直後に配置、coding-style.md § Cross-File Reference Lifecycle の具体化事例として cite、派生プロジェクト (techbook-ledger / auto-review-fix-vc) へ global 経由で自動波及、`feedback_global_config_backup` 適用必須) | | 207 | 💎 Tier 3 | **`~/.claude/rules/common/patterns.md` § Experimental Feature 設計時の参照必須 に「mechanical lint は ADR-039 scope 外」境界 case 追加 (PR #204 post-merge-feedback T3-2 採用)** | todo10.md | S | なし (PR #204 で project-local adr-039 § 1.b 追加した知見の global 投影、派生プロジェクトでの同型 over-application 防止、4 条件 (non-blocking / 決定論 / scope 限定 / recovery hint 明確) + 該当例 (file-length / file-size lint) と非該当例 (post-merge-feedback / weekly-review / local-llm) を境界 case として明示、順位 200/202/205 と同 pattern = project-local 知見の global codification、`feedback_global_config_backup` 適用必須) | -| 208 | 🔧 Tier 2 | **`drain_pipe_capped` 境界値テスト (N-1 / N / N+1 行) を lib-subprocess に追加 (PR #207 post-merge-feedback T2-1 採用)** | todo10.md | S | なし (PR #207 で CR が 🟠 Major として発見した pre-existing bug の root cause = `pr_size_check.rs` で `drain_pipe_capped(MAX_LINES=40)` が 40 行目以降の `jj diff --stat` summary line を silent truncate → `extract_summary_line()` が空文字を返し `pr_size_check` が silent disable (fail-open、ADR-043 violation) 化した事象を test レベルで予防、6 boundary test 追加で off-by-one regression を mechanical pin、順位 173e variant merge 検討の前段として variant 判断の test ベース根拠を強化) | **戦略**: 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/todo10.md b/docs/todo10.md index b2df2536..6793e334 100644 --- a/docs/todo10.md +++ b/docs/todo10.md @@ -432,49 +432,6 @@ ADR-039 (Experimental Feature 標準パターン) は「behavior の妥当性が --- -### `drain_pipe_capped` 境界値テスト (N-1 / N / N+1 行) を lib-subprocess に追加 (PR #207 post-merge-feedback T2-1 採用) - -> **動機**: PR #207 (順位 173c) で CR が 🟠 Major として発見した pre-existing bug の root cause = `pr_size_check.rs` の `drain_pipe_capped(MAX_LINES=40)` が `jj diff --stat` の summary line (40 行目以降) を silent truncate し、`extract_summary_line()` が空文字を返して `pr_size_check` が silent disable (fail-open、ADR-043 violation) する事象。PR #207 で追加した 6 テストは各 variant の代表 case を網羅するが、**MAX_LINES の boundary behavior (N-1 行 = 全件保持 / N 行 = 境界 / N+1 行 = 切り捨て初発生)** の直接検証は未実装。境界値の regression test を追加することで「定数を変えても挙動が読み取れる」「callsite で MAX_LINES を渡し間違えると test で検出」状態を確立する。 -> -> **本タスクの位置づけ**: PR #207 post-merge-feedback Tier 2 #1 採用 (Severity Medium / Frequency Low / Effort S / Adoption Risk None、2026-06-14 ユーザー承認)。analyzer rationale: 「CR が指摘した pre-existing bug の root cause を test レベルで予防、Effort S で Adoption Risk None」。順位 173e (variant merge 検討) の前段として実装することで variant 判断の test ベース根拠が強化される。 -> -> **参照**: `.claude/feedback-reports/207.md` Tier 2 #1、PR #207 CR Major comment (`drain_pipe_capped` silent truncate of control flow stdout)、`src/lib-subprocess/src/lib.rs` (3 variant 実装)、ADR-043 (Security/Quality Gate での Fail-Closed 原則 — silent disable は違反) -> -> **実行優先度**: 🔧 **Tier 2** — Effort S。既存 test の延長で 3-6 case 追加、PR diff < 100 行見込み。 - -#### 設計決定 (案) - -- **対象 variant**: `drain_pipe_capped` と `drain_pipe_capped_reporting` の両方 (truncate semantics が boundary を跨ぐ) -- **boundary 値 set** (各 variant): max_lines = 5 として: - - N-1 = 4 行 → 全件保持 (truncate 発生せず) - - N = 5 行 → 全件保持 (= cap と一致、truncate 発生せず) - - N+1 = 6 行 → 5 行保持 + 1 行切り捨て -- **`drain_pipe_capped_reporting` 特有**: - - N 行: truncated 報告なし (`"... (N lines truncated)"` 不在) - - N+1 行: truncated 報告あり (`"... (1 lines truncated)"` 末尾付与) -- **既存 test との関係**: 既存 6 test は短い (3-5 行) input で代表挙動を確認、新規 6 test は boundary 値で境界条件を pin -- **memory `feedback_test_dry_antipattern`**: 各 boundary case は独立 setup で記述、helper 関数で共通化しない - -#### 作業計画 - -- [ ] `src/lib-subprocess/src/lib.rs` の `#[cfg(test)] mod tests` 末尾に 6 boundary test を追加 (`drain_pipe_capped_N_minus_1_keeps_all` / `_N_keeps_all` / `_N_plus_1_truncates_one` + `drain_pipe_capped_reporting_*` 3 件) -- [ ] `cargo test -p lib-subprocess` で全 pass 確認 (既存 15 test + 新規 6 = 21 test) -- [ ] mutation regression check: 意図的に `< max_lines` を `<= max_lines` (off-by-one) に変えて新 test が落ちることを手動検証 -- [ ] cargo clippy clean -- [ ] 本エントリ削除 + docs/todo-summary.md 行削除 - -#### 完了基準 - -- 6 boundary test が追加され全 pass -- off-by-one regression (`< max_lines` ↔ `<= max_lines` mutation) で新 test が落ちる構造 -- `drain_pipe_capped_reporting` の truncated 報告 on/off boundary が test で明示的に確認 - -#### 詰まっている箇所 - -なし。Effort S、既存 test pattern (Cursor input + 直接 assertion) の延長で完結。 - ---- - ## 既知課題 (記録のみ、本セッションで未対応) (現時点で本ファイルへの既知課題は無し。docs/todo9.md 末尾を参照。) diff --git a/src/lib-subprocess/src/lib.rs b/src/lib-subprocess/src/lib.rs index 52953293..8da219fe 100644 --- a/src/lib-subprocess/src/lib.rs +++ b/src/lib-subprocess/src/lib.rs @@ -198,7 +198,8 @@ pub fn drain_pipe_capped_reporting( } } if truncated > 0 { - collected.push(format!("... ({} lines truncated)", truncated)); + let suffix = if truncated == 1 { "" } else { "s" }; + collected.push(format!("... ({} line{} truncated)", truncated, suffix)); } collected.join("\n") }) @@ -468,6 +469,51 @@ mod tests { assert_eq!(handle.join().unwrap(), "a\nb"); } + #[test] + fn drain_pipe_capped_n_minus_1_keeps_all() { + let input = Cursor::new(b"a\nb\nc\nd\n".to_vec()); + let handle = drain_pipe_capped(input, 5); + assert_eq!(handle.join().unwrap(), "a\nb\nc\nd"); + } + + #[test] + fn drain_pipe_capped_n_keeps_all() { + let input = Cursor::new(b"a\nb\nc\nd\ne\n".to_vec()); + let handle = drain_pipe_capped(input, 5); + assert_eq!(handle.join().unwrap(), "a\nb\nc\nd\ne"); + } + + #[test] + fn drain_pipe_capped_n_plus_1_truncates_one() { + let input = Cursor::new(b"a\nb\nc\nd\ne\nf\n".to_vec()); + let handle = drain_pipe_capped(input, 5); + assert_eq!(handle.join().unwrap(), "a\nb\nc\nd\ne"); + } + + #[test] + fn drain_pipe_capped_reporting_n_minus_1_keeps_all_omits_summary() { + let input = Cursor::new(b"a\nb\nc\nd\n".to_vec()); + let handle = drain_pipe_capped_reporting(input, 5); + assert_eq!(handle.join().unwrap(), "a\nb\nc\nd"); + } + + #[test] + fn drain_pipe_capped_reporting_n_keeps_all_omits_summary() { + let input = Cursor::new(b"a\nb\nc\nd\ne\n".to_vec()); + let handle = drain_pipe_capped_reporting(input, 5); + assert_eq!(handle.join().unwrap(), "a\nb\nc\nd\ne"); + } + + #[test] + fn drain_pipe_capped_reporting_n_plus_1_truncates_one_appends_summary() { + let input = Cursor::new(b"a\nb\nc\nd\ne\nf\n".to_vec()); + let handle = drain_pipe_capped_reporting(input, 5); + assert_eq!( + handle.join().unwrap(), + "a\nb\nc\nd\ne\n... (1 line truncated)", + ); + } + #[test] fn run_cmd_shell_capped_returns_true_on_exit_zero() { let (ok, _output) = run_cmd_shell_capped("test", "exit 0", 10, 40);