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
1 change: 0 additions & 1 deletion docs/todo-summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 への直接効果は小さい。

Expand Down
43 changes: 0 additions & 43 deletions docs/todo10.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 末尾を参照。)
48 changes: 47 additions & 1 deletion src/lib-subprocess/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
Expand Down Expand Up @@ -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);
Expand Down