Skip to content

fix(push-runner): bookmark_check の bookmark list 失敗を fail-closed にする (順位 288(b)) - #434

Merged
aloekun merged 1 commit into
masterfrom
fix/bookmark-check-fail-closed
Aug 21, 2026
Merged

fix(push-runner): bookmark_check の bookmark list 失敗を fail-closed にする (順位 288(b))#434
aloekun merged 1 commit into
masterfrom
fix/bookmark-check-fail-closed

Conversation

@aloekun

@aloekun aloekun commented Aug 21, 2026

Copy link
Copy Markdown
Owner

概要

不具合修正バックログ消化計画の PR I。順位 288 の残タスク (b) を扱う。

着手前に台帳と実装を突き合わせたところ、台帳が書いていた「@ の非 trunk 祖先が未レビューのまま push される穴」は (a) 側の実装で既に閉じていた。代わりに、同 stage の jj bookmark list 失敗経路が fail-open で残っていたため、そちらを塞ぐ (ユーザー判断)。

束ねた理由: 単一 crate・単一 stage の fail-closed 化 1 件。PR H と分けたのは、新しい中断経路の追加は誤 block リスクがあり、切り戻し単位を独立させるため (計画書どおり)。

台帳と実態のずれ

レビュー対象 diff の範囲は本 stage ではなく [diff] stage 側で既に閉じている:

  • config load 時に {{PR_RANGE}} の使用を必須化 (config::validate_diff_pr_range) — -r @ の直書き config は起動しない
  • 生成 diff が PR 範囲の全変更ファイルを含むかを verify_diff_covers_pr_range が fail-closed で検査
  • 範囲は top-level default_branch の単一真実源から解決

よって祖先コミットごとの review 証跡照合 gate を新設すると判定が二重化し、誤 block リスクだけが増えるため不採用とした。この判断根拠は bookmark_check.rs の module doc に残している (計画書は ephemeral なため)。

本計画で着手した 10 件中 8 件目のずれ。

実在した穴

detect_own_workspace_bookmarksjj bookmark list の失敗時に Some(空) を返して push を続行していた。この 1 経路が本 stage の判定を丸ごと迂回する:

  • 空リストでは push stage の build_push_command-b <name> を組み立てられず、base command (jj git push) がそのまま実行される。jj 0.42 の既定は tracked bookmark を全件送るため、レビュー範囲 (<default_branch>..@) 外の ref が AI レビューを経ずに送られる。ADR-045 事故で --all を廃止したのと同じ経路
  • @ の空判定・description 判定 (HeadState::Unknown / DescUnknown を fail-closed に倒す分岐) は list 成功後にしか走らないため、この経路では一度も評価されない
  • 発火条件は現実的で、並列 workspace 運用の jj lock 競合による 30s timeout は diff stage が T6 で塞いだものと同クラス
  • この経路のテストは 1 本も無かった

実データ上の裏付け: 本 repo には現在 claude/nightly-228 / claude/nightly-324 / claude/nightly-383 の 3 本が tracked bookmark として存在し、fail-open 経路に落ちればこれらが巻き込まれる構成にある。

対処

  • 失敗を BookmarkCheckOutcome::BookmarkListUnavailable に落として None を返す (pipeline 中断 / ADR-043)。env override は設けない (ユーザー判断。同モジュールの他の fail-closed 判定と揃える)
  • decide_from_bookmark_list を切り出し、head_state を closure で受けて list 失敗時に @ の状態を照会しない (中断は確定しており、不調な jj を叩いても案内は変わらない。照会結果で案内を出し分けると「@ が空です」等の事実と異なる案内を再生産する余地が残る)
  • 案内は「なぜ止めるか」= 実害を述べる。jj エラーの転記だけだと override 手段を探す方向に人が動く
  • 戻り値の不変条件 (Some のとき必ず 1 件以上) を doc 化し、push stage 側の空リスト fallback が派生プロジェクト config 専用の経路になったことを追記

回帰テスト

rank288b_bookmark_list_failure 5 件:

テスト 固定する内容
list_failure_aborts_instead_of_proceeding_with_empty_list incident 再現 (bad): 失敗 → 中断
list_success_still_proceeds 対 (good): 成功 → 従来どおり Proceed
list_failure_does_not_query_head_state 失敗時に @ を照会しない
hint_states_the_actual_harm_not_just_the_jj_error 案内が実害を述べる
proceed_never_carries_an_empty_bookmark_list Some の非空不変条件

副次

  • report_outcome が 61 行になり関数長 gate に触れたため abort_report を分離
  • ファイル長 800 行 gate に触れたため test mod を bookmark_check/tests.rs へ切り出し (diff stage と同じ形)
  • pre-push simplicity review の非ブロッキング警告 2 件に対応: module doc の「中断理由は 2 ケース」が実態とずれていた点、および同じ diff で新設した abort_report の doc が自ら定めた「件数を書かない」方針を破っていた点

後始末

docs/todo15.md 順位 288 節 + docs/todo-summary2.md 288 行を削除。計画書 docs/bugfix-batch-plan.md の進行表更新はマージ後の docs バッチで行う (PR H → #433 と同じ運用)。

検証

  • cargo test --workspace green (全 40 スイート)
  • cargo clippy -p cli-push-runner --all-targets green
  • pre-push review: simplicity / security とも approved

Summary by CodeRabbit

  • 機能改善

    • ブックマーク情報の取得に失敗した場合、誤った範囲への push を防ぐため処理を中止するよう変更しました。
    • 判定結果に応じた案内メッセージを改善しました。
  • ドキュメント

    • ブックマークが空になる条件と、検査失敗時の動作を追記しました。
  • テスト

    • ブックマーク解析、状態判定、取得失敗時の中止処理などの検証を追加しました。

…(順位 288(b))

順位 288 の残タスク (b)。着手前に台帳と実装を突き合わせたところ、台帳が書いていた
「@ の非 trunk 祖先が未レビューのまま push される穴」は (a) 側の実装で既に閉じて
いた。代わりに、同 stage の bookmark list 失敗経路が fail-open で残っていた。

台帳と実態のずれ:
- レビュー対象 diff の範囲は [diff] stage 側で閉じている。config load 時に
  PR_RANGE プレースホルダの使用を必須化 (validate_diff_pr_range) し、生成 diff が
  PR 範囲の全変更ファイルを含むかを verify_diff_covers_pr_range が fail-closed で
  検査する
- よって祖先コミットごとの review 証跡照合 gate を新設すると判定が二重化し、
  誤 block リスクだけが増える (ユーザー判断で不採用)

実在した穴 (本 PR の修正対象):
- detect_own_workspace_bookmarks が bookmark list の失敗時に Some(空) を返して
  push を続行していた。この 1 経路が本 stage の判定を丸ごと迂回する
- 空リストでは push stage の build_push_command が -b <name> を組み立てられず
  base command がそのまま実行される。jj 0.42 の既定は tracked bookmark を全件
  送るため、レビュー範囲 (<default_branch>..@) 外の ref (他 workspace / 夜間ループの
  claude/nightly-* 等) が AI レビューを経ずに送られる。ADR-045 事故で --all を
  廃止したのと同じ経路
- @ の空判定 / description 判定 (HeadState::Unknown / DescUnknown を fail-closed に
  倒す分岐) は list 成功後にしか走らないため、この経路では一度も評価されない
- 発火条件は現実的で、並列 workspace 運用の jj lock 競合による 30s timeout は
  diff stage が T6 で塞いだものと同クラス

対処:
- 失敗を BookmarkCheckOutcome::BookmarkListUnavailable に落として None を返す
  (pipeline 中断 / ADR-043)。env override は設けない (ユーザー判断。同モジュールの
  他の fail-closed 判定と揃える)
- decide_from_bookmark_list を切り出し、head_state を closure で受けて list 失敗時に
  @ の状態を照会しない (中断は確定しており、不調な jj を叩いても案内は変わらない)
- 案内は「なぜ止めるか」= 実害を述べる。jj エラーの転記だけだと override 手段を
  探す方向に人が動く
- 戻り値の不変条件 (Some のとき必ず 1 件以上) を doc 化し、push stage 側の
  空リスト fallback が派生プロジェクト config 専用の経路になったことを追記

回帰テスト 5 件 (rank288b_bookmark_list_failure): list 失敗 → 中断 / list 成功 →
従来どおり Proceed の対、head state を照会しない、案内が実害を述べる、
Proceed が空リストを運ばない

副次:
- report_outcome が 61 行になり関数長 gate に触れたため abort_report を分離
- ファイル長 800 行 gate に触れたため test mod を bookmark_check/tests.rs へ
  切り出し (diff stage と同じ形)

後始末: todo15.md 順位 288 節 + todo-summary2.md 288 行を削除

検証: cargo test --workspace green / cargo clippy -p cli-push-runner --all-targets green
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 868ea69b-3059-48d8-a837-42bbb04fccce

📥 Commits

Reviewing files that changed from the base of the PR and between e3e7ae7 and 73c33d4.

📒 Files selected for processing (5)
  • docs/todo-summary2.md
  • docs/todo15.md
  • src/cli-push-runner/src/stages/bookmark_check.rs
  • src/cli-push-runner/src/stages/bookmark_check/tests.rs
  • src/cli-push-runner/src/stages/push.rs
💤 Files with no reviewable changes (2)
  • docs/todo-summary2.md
  • docs/todo15.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

bookmark_checkjj bookmark list の失敗時に fail-closed で push を停止します。判定結果、案内文、テスト、push の説明、関連 TODO を更新しました。

Changes

bookmark_check の失敗時停止

Layer / File(s) Summary
bookmark 判定と中断結果
src/cli-push-runner/src/stages/bookmark_check.rs
bookmark 一覧の取得失敗を BookmarkListUnavailable として扱います。失敗時は HEAD 状態を照会せず、push を停止します。中断理由と再実行条件を報告します。
回帰検証と関連文書
src/cli-push-runner/src/stages/bookmark_check/tests.rs, src/cli-push-runner/src/stages/push.rs, docs/todo-summary2.md, docs/todo15.md
bookmark 解析、空の working copy、親リビジョン、取得失敗時の fail-closed 動作を検証します。push 条件の説明と完了済み TODO を更新します。

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 73c33

The change stops the push stage when bookmark discovery fails and adds focused regression coverage; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant bookmark_check
  participant jj
  participant push_pipeline
  bookmark_check->>jj: jj bookmark list を実行
  jj-->>bookmark_check: bookmark 一覧または実行エラー
  bookmark_check->>push_pipeline: 成功結果を返す、または中断する
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed タイトルは、bookmark_checkjj bookmark list 失敗時を fail-closed に変更する主な修正内容を明確に示しています。
Docstring Coverage ✅ Passed Docstring coverage is 84.78% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 46 functions across 3 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/bookmark-check-fail-closed

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 PR Monitor 分析 (GitHub Actions バックストップ)

  • トリガー: issue_comment (created) / 実行 run
  • CI: rust (ubuntu-latest) pending / rust (windows-latest) pending / request skipping — 全 check 未確定
  • レビュー状況: CodeRabbit は「10 stars 未満の OSS リポジトリのため自動レビュー対象外 (manual trigger 必要)」の定型通知のみ (review 0 件)。人間レビューも 0 件。インライン指摘 0 件。→ 未実施 (陽性証拠なし)
  • Verdict: user_decision

Applicable Findings (Critical / High / Major)

(該当なし — レビュー未実施)

Applicable Findings (Medium 以下)

(該当なし)

Filtered (not applicable)

(該当なし)

次のアクション

  • CodeRabbit レビューが自動発火しないリポジトリ設定 (10 stars 未満) のため、必要なら @coderabbitai review で手動トリガーするか、人間レビューで代替する。
  • CI (rust (ubuntu-latest) / (windows-latest)) が pending のため、完了後に結果を確認する。
  • 変更は bookmark_check.rs を fail-open → fail-closed に倒す設計変更 (順位 288(b) / ADR-043 準拠) とテスト・todo 台帳更新のみで、diff 自体は自己完結している。レビュー証拠が揃うまでマージ判断を保留。

@aloekun
aloekun merged commit 661b9f9 into master Aug 21, 2026
4 checks passed
@aloekun
aloekun deleted the fix/bookmark-check-fail-closed branch August 21, 2026 09:34
aloekun added a commit that referenced this pull request Aug 21, 2026
着手前に台帳と実装を突き合わせた。台帳が推定していた根因「pre-push の
[diff] stage が jj diff -r @ を使う」は PR #311 で解消済みで、実在する根因は
別の 2 箇所だった。

台帳と実態のずれ:
- [diff] stage は PR #311 で {{PR_RANGE}} プレースホルダ必須化 + 範囲カバレッジ
  検査済み。config load 時に revset 直書きを reject するため、この経路は閉じている

実在した根因 2 件:
- (i) .takt/facets/instructions/fix.md の refresh が `jj diff -r @` (tip 限定、
  かつ --git なし) で review-diff.txt を上書きしていた。fix を挟んだ iteration は
  祖先コミットが見えない diff を読む。pre-push / post-pr の両経路に影響し、
  .claude/feedback-reports/348.md で実バグとして指摘されたが台帳未起票だった
- (ii) post-pr-review の analyze は push の後に走るため、pre-push run が diff を
  書かなかった場合 (DiffResult::Empty で takt skip 等) review-diff.txt に別 PR の
  残骸が残る。実測: PR #434 マージ後も同ファイルに #434 の diff (5 ファイル) が
  残存していた

どちらも「code を含む PR」を docs-only と誤判定させ、CodeRabbit が PR 全体を見て
出した finding を ADR-035 filter で握り潰す (PR #227 実観測)。

対処 (ユーザー判断: 決定論化):
- cli-pr-monitor が GitHub の PR 全体ファイル一覧 (gh pr view --json files) を
  lib_docs_policy で分類し、.takt/review-comments.json に docs_only として書く。
  ローカル working copy 状態に一切依存しないため (i)(ii) が構造的に消える
- lib-docs-policy に paths 版 API (is_docs_only_paths) を追加。status を持たない
  source からの入口で、判定規則は is_docs_only_path の単一実装に集約したまま
  (ADR-035 が防ごうとした drift の再生産を避ける)
- fail-closed (ADR-043): 一覧を取得できなければ docs_only=false。docs-only 扱いは
  filter を緩める方向なので、取得失敗を true に倒すと finding を握り潰す
- analyze-coderabbit.md は docs_only を読むだけにし、review-diff.txt を見ない。
  フィールド不在時は false 扱い (exe 更新前の JSON に対する後方互換、ADR-051)

fix.md の refresh も同時に修正 (ユーザー判断):
- `jj diff --git -r 'trunk()..@'` に変更。trunk() は jj builtin でリモート trunk を
  解決するため、default branch が main の派生プロジェクトでもそのまま動く
- 実測 (read-only): 2 コミット範囲で tip-only 5 ファイル vs 範囲 9 ファイル。
  code ファイル src/cli-docs-lint/src/preamble.rs を含む 4 件が tip-only から
  見えないことを確認した

回帰テスト:
- lib-docs-policy rank233_paths_entry_point 5 件 (docs のみ / 混在 / 除外パス /
  空入力の fail-closed / summary 版との規則一致)
- cli-pr-monitor rank233_docs_only_verdict 4 件 (incident 再現の混在ケース /
  docs のみの対照 / 取得失敗の fail-closed / 除外パスの委譲確認)

後始末: todo13.md 233 節 + todo-summary2.md 233 行を削除

検証: cargo test --workspace green / cargo clippy --workspace --all-targets green /
pnpm lint:docs green / pnpm lint:md green
aloekun added a commit that referenced this pull request Aug 21, 2026
着手前に台帳と実装を突き合わせた。台帳が推定していた根因「pre-push の
[diff] stage が jj diff -r @ を使う」は PR #311 で解消済みで、実在する根因は
別の 2 箇所だった。

台帳と実態のずれ:
- [diff] stage は PR #311 で {{PR_RANGE}} プレースホルダ必須化 + 範囲カバレッジ
  検査済み。config load 時に revset 直書きを reject するため、この経路は閉じている

実在した根因 2 件:
- (i) .takt/facets/instructions/fix.md の refresh が `jj diff -r @` (tip 限定、
  かつ --git なし) で review-diff.txt を上書きしていた。fix を挟んだ iteration は
  祖先コミットが見えない diff を読む。pre-push / post-pr の両経路に影響し、
  .claude/feedback-reports/348.md で実バグとして指摘されたが台帳未起票だった
- (ii) post-pr-review の analyze は push の後に走るため、pre-push run が diff を
  書かなかった場合 (DiffResult::Empty で takt skip 等) review-diff.txt に別 PR の
  残骸が残る。実測: PR #434 マージ後も同ファイルに #434 の diff (5 ファイル) が
  残存していた

どちらも「code を含む PR」を docs-only と誤判定させ、CodeRabbit が PR 全体を見て
出した finding を ADR-035 filter で握り潰す (PR #227 実観測)。

対処 (ユーザー判断: 決定論化):
- cli-pr-monitor が GitHub の PR 全体ファイル一覧を lib_docs_policy で分類し、
  .takt/review-comments.json に docs_only として書く。ローカル working copy 状態に
  一切依存しないため (i)(ii) が構造的に消える
- lib-docs-policy に paths 版 API (is_docs_only_paths) を追加。status を持たない
  source からの入口で、判定規則は is_docs_only_path の単一実装に集約したまま
  (ADR-035 が防ごうとした drift の再生産を避ける)
- fail-closed (ADR-043): 一覧を取得できなければ docs_only=false。docs-only 扱いは
  filter を緩める方向なので、取得失敗を true に倒すと finding を握り潰す
- analyze-coderabbit.md は docs_only を読むだけにし、review-diff.txt を見ない。
  フィールド不在時は false 扱い (exe 更新前の JSON に対する後方互換、ADR-051)

fix.md の refresh も同時に修正 (ユーザー判断):
- `jj diff --git -r 'trunk()..@'` に変更。trunk() は jj builtin でリモート trunk を
  解決するため、default branch が main の派生プロジェクトでもそのまま動く
- 実測 (read-only): 2 コミット範囲で tip-only 5 ファイル vs 範囲 9 ファイル。
  code ファイル src/cli-docs-lint/src/preamble.rs を含む 4 件が tip-only から
  見えないことを確認した

CodeRabbit Major 対応 (自分が作った gate 自身の fail-open):
- 初版は `gh pr view --json files` で一覧を取っていたが、これは 100 件で無言に
  切り捨てる (cli/cli#13338)。先頭 100 件が docs、101 件目が source の PR で
  docs_only=true に倒れ、本 PR が塞ごうとしている誤フィルタを再生産していた
- 実測 (rust-lang/rust#161453、185 ファイル): changedFiles=185 に対し
  `gh pr view --json files` は 100 件、`gh api --paginate .../files` は 185 件
- REST files endpoint を --paginate で辿る方式に変更し、加えて取得件数と PR 申告の
  changedFiles の一致を要求する。上限値 (100 / REST の 3,000) を定数に持たないのは、
  上限が変わっても件数一致検査が成立し続けるようにするため
- 変異テストで確認: 件数一致 guard を外すと該当テストが FAILED になる

回帰テスト:
- lib-docs-policy rank233_paths_entry_point 6 件 (docs のみ / 混在 / 除外パス /
  空入力の fail-closed / summary 版を委譲させない理由の seal / 規則一致)
- cli-pr-monitor rank233_docs_only_verdict 6 件 (incident 再現の混在ケース /
  docs のみの対照 / 取得失敗の fail-closed / 除外パスの委譲確認 /
  切り捨て一覧の検出 / 件数一致時は従来どおり判定する対照)

後始末: todo13.md 233 節 + todo-summary2.md 233 行を削除

検証: cargo test --workspace green / cargo clippy --workspace --all-targets green /
pnpm lint:docs green / pnpm lint:md green
aloekun added a commit that referenced this pull request Aug 21, 2026
)

着手前に台帳と実装を突き合わせた。台帳が推定していた根因「pre-push の
[diff] stage が jj diff -r @ を使う」は PR #311 で解消済みで、実在する根因は
別の 2 箇所だった。

台帳と実態のずれ:
- [diff] stage は PR #311 で {{PR_RANGE}} プレースホルダ必須化 + 範囲カバレッジ
  検査済み。config load 時に revset 直書きを reject するため、この経路は閉じている

実在した根因 2 件:
- (i) .takt/facets/instructions/fix.md の refresh が `jj diff -r @` (tip 限定、
  かつ --git なし) で review-diff.txt を上書きしていた。fix を挟んだ iteration は
  祖先コミットが見えない diff を読む。pre-push / post-pr の両経路に影響し、
  .claude/feedback-reports/348.md で実バグとして指摘されたが台帳未起票だった
- (ii) post-pr-review の analyze は push の後に走るため、pre-push run が diff を
  書かなかった場合 (DiffResult::Empty で takt skip 等) review-diff.txt に別 PR の
  残骸が残る。実測: PR #434 マージ後も同ファイルに #434 の diff (5 ファイル) が
  残存していた

どちらも「code を含む PR」を docs-only と誤判定させ、CodeRabbit が PR 全体を見て
出した finding を ADR-035 filter で握り潰す (PR #227 実観測)。

対処 (ユーザー判断: 決定論化):
- cli-pr-monitor が GitHub の PR 全体ファイル一覧を lib_docs_policy で分類し、
  .takt/review-comments.json に docs_only として書く。ローカル working copy 状態に
  一切依存しないため (i)(ii) が構造的に消える
- lib-docs-policy に paths 版 API (is_docs_only_paths) を追加。status を持たない
  source からの入口で、判定規則は is_docs_only_path の単一実装に集約したまま
  (ADR-035 が防ごうとした drift の再生産を避ける)
- fail-closed (ADR-043): 一覧を取得できなければ docs_only=false。docs-only 扱いは
  filter を緩める方向なので、取得失敗を true に倒すと finding を握り潰す
- analyze-coderabbit.md は docs_only を読むだけにし、review-diff.txt を見ない。
  フィールド不在時は false 扱い (exe 更新前の JSON に対する後方互換、ADR-051)

fix.md の refresh も同時に修正 (ユーザー判断):
- `jj diff --git -r 'trunk()..@'` に変更。trunk() は jj builtin でリモート trunk を
  解決するため、default branch が main の派生プロジェクトでもそのまま動く
- 実測 (read-only): 2 コミット範囲で tip-only 5 ファイル vs 範囲 9 ファイル。
  code ファイル src/cli-docs-lint/src/preamble.rs を含む 4 件が tip-only から
  見えないことを確認した

CodeRabbit Major 対応 (自分が作った gate 自身の fail-open):
- 初版は `gh pr view --json files` で一覧を取っていたが、これは 100 件で無言に
  切り捨てる (cli/cli#13338)。先頭 100 件が docs、101 件目が source の PR で
  docs_only=true に倒れ、本 PR が塞ごうとしている誤フィルタを再生産していた
- 実測 (rust-lang/rust#161453、185 ファイル): changedFiles=185 に対し
  `gh pr view --json files` は 100 件、`gh api --paginate .../files` は 185 件
- REST files endpoint を --paginate で辿る方式に変更し、加えて取得件数と PR 申告の
  changedFiles の一致を要求する。上限値 (100 / REST の 3,000) を定数に持たないのは、
  上限が変わっても件数一致検査が成立し続けるようにするため
- 変異テストで確認: 件数一致 guard を外すと該当テストが FAILED になる

回帰テスト:
- lib-docs-policy rank233_paths_entry_point 6 件 (docs のみ / 混在 / 除外パス /
  空入力の fail-closed / summary 版を委譲させない理由の seal / 規則一致)
- cli-pr-monitor rank233_docs_only_verdict 6 件 (incident 再現の混在ケース /
  docs のみの対照 / 取得失敗の fail-closed / 除外パスの委譲確認 /
  切り捨て一覧の検出 / 件数一致時は従来どおり判定する対照)

後始末: todo13.md 233 節 + todo-summary2.md 233 行を削除

検証: cargo test --workspace green / cargo clippy --workspace --all-targets green /
pnpm lint:docs green / pnpm lint:md green
aloekun added a commit that referenced this pull request Aug 22, 2026
不具合修正バックログ消化計画 (PR I-L = #434 / #435 / #436 / #437) の post-merge
feedback 全 48 提案を採否判定した。内訳は採用候補 21 / 様子見 11 / 却下推奨 12、
および実コード確認で 1 件脱落。

ユーザー判断 (2026-08-22):
- Tier 1 (決定論的防止) は 4 件すべて採用
- Tier 2 (テスト/自動化) は実装の穴埋めに直結する 5 件を採用
- Tier 3 (ドキュメント/ルール) は 8 件すべて却下

T3 却下の根拠は本 feedback 自身が示した実証にある。PR #438 の feedback が
「routing 更新チェックリストは既に docs/dev-conventions.md に存在したのに
3 件目の再発を防げなかった」と指摘しており、規約追記の有効性が否定的に
実証された。同じ形の 8 件を足す理由が無い。内容は各 PR の doc コメントと
PR 本文に記録済みで、失われるものは無い。

起票 (統合の単位は「そのまま 1 PR になる粒度」):
- 481 (T1): lib-subprocess の失敗経路を塞ぎ切る。#436 T1-1 は実バグで、正常終了
  経路の join だけが join_within_grace を経由せず無制限のまま残っている
  (実コードで現存を確認済み)。T2-1/T2-3 のテスト補強を同じ単位に含める
- 482 (T1): 外部コマンド呼び出しの落とし穴を lint で塞ぐ。gh の 100 件無言
  切り捨てと git push --force の lease 欠落。どちらも今回実際に踏んだ
- 483 (T2): エラーメッセージの無制限 debug 補間を lint で検出する
- 484 (T2): push stage の bare push フォールバック不変条件を seal する
- 485 (T2): PR L で追加した実装のテスト補強

起票前の実コード確認で 1 件が脱落した:
- #437 T1-4「parse エラーに行番号 + 行の中身」は PR L の D-2 で実装済みだった
  (SourceLine / clip_for_message を確認)。同じ確認で前回も 1 件脱落しており、
  feedback レポートは台帳と同じく実装が動くほどずれる

採用 9 件のうち 4 件が「テストが一部の経路しか通っていなかった」形で、本セッション
中に 2 度踏んだテストの空振りと同型。

PR #426 の failed marker も復旧した (pnpm merge-pr --feedback-only 426)。全 7 提案の
採用候補 1 件は T3 のため上記方針に従い却下。docs 変更は生じない。

検証: pnpm lint:docs green / pnpm lint:md green

CodeRabbit 指摘 4 件に対応 (PR #439、いずれも妥当):
- Minor: 採否件数が合っていなかった (21+11+12+1=45≠48)。実数を数え直すと表に載った
  36 件 (採用 21 / 様子見 7 / 却下 8) + 除外 4 件 = 40 件。「48」は前回バッチ (PR E-H) の
  数字を数え直さず流用したもので、レポートを機械的に数えれば 5 秒で分かる値だった。
  再発防止として「件数は数え直すこと」を節の前書きに明記した
- Major (順位 481): 正常終了経路の無制限 join を「上限を入れるか、入れない理由を doc に
  記録する」と両論併記していたが、**文書化では hang を 1 ミリ秒も縮められない**。上限付きを
  必須とし、子孫がパイプを握ったまま子が正常終了するケースの決定論的テストを完了基準に加えた
- Major (順位 482): lease を要求する対象が todo25.md では削除系 (--delete)、summary2 では
  非 fast-forward 更新系 (--force) とずれていた。**両者は同じ lint パターンでは捕まらず**、
  --force だけを見る規則では削除経路が丸ごと素通りする (PR L で実際に踏んだのは削除系)。
  refspec 形式 (:refs/... / +refs/...) も含めて 2 種類を表で明示し、両文書を統一した
- Major (順位 485): inject_git_dir_for_gh_with は GIT_DIR と cwd という**プロセス全体状態**を
  読み書きするため、テスト並列実行で他テストと競合する。Drop guard による復元 (ADR-025 の
  CwdRestore が前例、GIT_DIR は「未設定」も状態として区別) と共有 mutex での直列化
  (ADR-041) を先行タスクとして追加し、完了基準に「並列 / 直列の両方で green」を加えた
aloekun added a commit that referenced this pull request Aug 22, 2026
不具合修正バックログ消化計画 (PR I-L = #434 / #435 / #436 / #437) の post-merge
feedback 全 48 提案を採否判定した。内訳は採用候補 21 / 様子見 11 / 却下推奨 12、
および実コード確認で 1 件脱落。

ユーザー判断 (2026-08-22):
- Tier 1 (決定論的防止) は 4 件すべて採用
- Tier 2 (テスト/自動化) は実装の穴埋めに直結する 5 件を採用
- Tier 3 (ドキュメント/ルール) は 8 件すべて却下

T3 却下の根拠は本 feedback 自身が示した実証にある。PR #438 の feedback が
「routing 更新チェックリストは既に docs/dev-conventions.md に存在したのに
3 件目の再発を防げなかった」と指摘しており、規約追記の有効性が否定的に
実証された。同じ形の 8 件を足す理由が無い。内容は各 PR の doc コメントと
PR 本文に記録済みで、失われるものは無い。

起票 (統合の単位は「そのまま 1 PR になる粒度」):
- 481 (T1): lib-subprocess の失敗経路を塞ぎ切る。#436 T1-1 は実バグで、正常終了
  経路の join だけが join_within_grace を経由せず無制限のまま残っている
  (実コードで現存を確認済み)。T2-1/T2-3 のテスト補強を同じ単位に含める
- 482 (T1): 外部コマンド呼び出しの落とし穴を lint で塞ぐ。gh の 100 件無言
  切り捨てと git push --force の lease 欠落。どちらも今回実際に踏んだ
- 483 (T2): エラーメッセージの無制限 debug 補間を lint で検出する
- 484 (T2): push stage の bare push フォールバック不変条件を seal する
- 485 (T2): PR L で追加した実装のテスト補強

起票前の実コード確認で 1 件が脱落した:
- #437 T1-4「parse エラーに行番号 + 行の中身」は PR L の D-2 で実装済みだった
  (SourceLine / clip_for_message を確認)。同じ確認で前回も 1 件脱落しており、
  feedback レポートは台帳と同じく実装が動くほどずれる

採用 9 件のうち 4 件が「テストが一部の経路しか通っていなかった」形で、本セッション
中に 2 度踏んだテストの空振りと同型。

PR #426 の failed marker も復旧した (pnpm merge-pr --feedback-only 426)。全 7 提案の
採用候補 1 件は T3 のため上記方針に従い却下。docs 変更は生じない。

検証: pnpm lint:docs green / pnpm lint:md green

CodeRabbit 指摘 4 件に対応 (PR #439、いずれも妥当):
- Minor: 採否件数が合っていなかった (21+11+12+1=45≠48)。実数を数え直すと表に載った
  36 件 (採用 21 / 様子見 7 / 却下 8) + 除外 4 件 = 40 件。「48」は前回バッチ (PR E-H) の
  数字を数え直さず流用したもので、レポートを機械的に数えれば 5 秒で分かる値だった。
  再発防止として「件数は数え直すこと」を節の前書きに明記した
- Major (順位 481): 正常終了経路の無制限 join を「上限を入れるか、入れない理由を doc に
  記録する」と両論併記していたが、**文書化では hang を 1 ミリ秒も縮められない**。上限付きを
  必須とし、子孫がパイプを握ったまま子が正常終了するケースの決定論的テストを完了基準に加えた
- Major (順位 482): lease を要求する対象が todo25.md では削除系 (--delete)、summary2 では
  非 fast-forward 更新系 (--force) とずれていた。**両者は同じ lint パターンでは捕まらず**、
  --force だけを見る規則では削除経路が丸ごと素通りする (PR L で実際に踏んだのは削除系)。
  refspec 形式 (:refs/... / +refs/...) も含めて 2 種類を表で明示し、両文書を統一した
- Major (順位 485): inject_git_dir_for_gh_with は GIT_DIR と cwd という**プロセス全体状態**を
  読み書きするため、テスト並列実行で他テストと競合する。Drop guard による復元 (ADR-025 の
  CwdRestore が前例、GIT_DIR は「未設定」も状態として区別) と共有 mutex での直列化
  (ADR-041) を先行タスクとして追加し、完了基準に「並列 / 直列の両方で green」を加えた
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant