feat(weekly-review): クローズ済み PR の残存ブランチを決定論的に検出し削除を提案する - #377
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughremote ブランチと PR の全状態を取得する Rust CLI を追加しました。closed または merged のみのブランチを削除候補として Markdown 出力します。削除は実行しません。L3 から同期実行できるようにビルドと週次パイプラインを更新しました。 Changes残存ブランチ検出
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator as 実行環境
participant ScanCLI as cli-stale-branch-scan
participant Git as git ls-remote
participant GH as gh pr list
participant Report as Markdown レポート
Operator->>ScanCLI: pnpm stale-branch-scan
ScanCLI->>Git: remote ブランチを取得
ScanCLI->>GH: ブランチごとの PR 状態を取得
ScanCLI->>Report: 削除候補と保護対象を出力
Report-->>Operator: 手動削除コマンドを提示
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし (レビュー指摘 0 件) Applicable Findings (Medium 以下)該当なし Filtered (not applicable)該当なし 次のアクション
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/cli-stale-branch-scan/src/main.rs (2)
169-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winconfig の読み取り失敗を stderr へ通知することを検討してください。
L170 と L176 は、ファイル欠如と TOML parse 失敗の両方を
Noneへ落とします。fail-closed にしない判断は doc (L164-168) のとおり妥当です。ただし parse 失敗は設定の破損を示します。現状は無音で
TRUNK_BOOKMARKSのみの保護へ縮退します。default_branchにTRUNK_BOOKMARKS外の名前を使うリポジトリでは、その trunk が削除提案に載ります。parse 失敗時のみ stderr へ 1 行出すと、レポートの決定性を保ったまま原因が分かります。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli-stale-branch-scan/src/main.rs` around lines 169 - 178, Update parse_configured_trunk_branch to emit one concise diagnostic line to stderr when TOML deserialization fails, while still returning None and preserving the existing fallback behavior. Keep missing-file handling in configured_trunk_branch silent, and ensure valid configurations remain unchanged.
272-277: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
remoteも同じ allowlist を通すことを検討してください。L273 は
remoteを削除コマンド文字列へそのまま埋め込みます。branchはis_safe_branch_nameで検査しますが、remoteは検査しません。L213 の header も同様です。現状
remoteは CLI 引数であり、実行者が制御します。したがって今は脅威ではありません。ただし将来 skill や workflow が外部由来の値を--remoteへ渡すと、branchと同じコピペ実行リスクが生じます。出力の全経路が 1 つの allowlist を通る形にすると、この差分が残りません。doc コメント (L244-246) が述べる方針とも一致します。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli-stale-branch-scan/src/main.rs` around lines 272 - 277, Validate the remote value with the same allowlist used by is_safe_branch_name before embedding it in generated delete commands and the header output. Update the command-generation flow around command_cell and the header construction so every output path handles unsafe remote values consistently, preserving the existing warning behavior and documented safety policy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/adr-031-weekly-review-pipeline.md`:
- Line 210: ADR-031のブランチ削除コマンド記述を、実装仕様に合わせて `git push origin --delete --
<branch>` の形式へ更新してください。対象は「削除は提案までで止める」節のコマンド表記のみとし、その他の説明は変更しないでください。
- Around line 132-145: Implement the missing /weekly-review L3 skill flow, using
the /monthly-review skill and its cli-telemetry-report execution path as the
reference. Run pnpm stale-branch-scan synchronously before starting takt,
include its output with the findings for the approval selection, and stop before
approval if the scan fails. Ensure the skill supports resuming from pending JSON
as described by the ADR.
In `@src/cli-stale-branch-scan/src/collect.rs`:
- Around line 159-171: Update fetch_pull_requests_for_branch so errors from
run("gh", &args) are also wrapped with the branch name before propagation, while
preserving the existing branch-context mapping for parse_pr_list errors.
---
Nitpick comments:
In `@src/cli-stale-branch-scan/src/main.rs`:
- Around line 169-178: Update parse_configured_trunk_branch to emit one concise
diagnostic line to stderr when TOML deserialization fails, while still returning
None and preserving the existing fallback behavior. Keep missing-file handling
in configured_trunk_branch silent, and ensure valid configurations remain
unchanged.
- Around line 272-277: Validate the remote value with the same allowlist used by
is_safe_branch_name before embedding it in generated delete commands and the
header output. Update the command-generation flow around command_cell and the
header construction so every output path handles unsafe remote values
consistently, preserving the existing warning behavior and documented safety
policy.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8491bb38-80c5-4a59-bca7-23b0cd9418b1
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomldocs/adr/adr-031-weekly-review-pipeline.mddocs/harness-improvement-plan.mddocs/todo-summary2.mddocs/todo21.mdpackage.jsonsrc/cli-stale-branch-scan/Cargo.tomlsrc/cli-stale-branch-scan/src/classify.rssrc/cli-stale-branch-scan/src/collect.rssrc/cli-stale-branch-scan/src/main.rs
💤 Files with no reviewable changes (1)
- docs/todo-summary2.md
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし Applicable Findings (Medium 以下)
Filtered (not applicable)
次のアクション
|
todo 順位 395。クローズ済み PR #365 のブランチを手で消したことで ADR-072 決定 3 の 除外マーカーが失われ、同じ順位が再選択された (PR #373)。決定 3 自体は設計どおりで、 ブランチの存在が着手済みマーカーである以上、浮いたブランチを定期的に片付ける場が要る。 **takt workflow には置けない。** 検出には git ls-remote / gh = ネットワークが要るが、 weekly-review.yaml は全 provider に network_access: false を課している (他 3 パイプラインは true)。1 つの scan のためにこれを反転すると whole-tree review 6 facet すべての隔離が 緩むため、/monthly-review が cli-telemetry-report を呼ぶのと同じく skill 側 (ADR-031 の L3 = 決定論層) に置いた。置き場所の判断基準 (ネットワークが要るか) を ADR-031 へ記録。 判定規則: - 紐づく PR がすべて closed/merged → 削除候補 - open が 1 本でもある → 対象外 (close 後に別 PR を開く / reopen が実在する) - PR が 1 件も無い → 対象外 (PR 未作成の作業中ブランチと区別できない) - trunk は常に対象外 / claude/nightly-* は除外しない (除外すると順位が二度と選ばれない) - state が未知の値 → open 扱い (保護側。誤って削除提案に載せない) **PR は全件引かずブランチごとに --head で引く。** 総 PR 数は単調増加する一方、remote ブランチ数は運用上小さく有界。全件方式は実装中に実際に破綻した (本リポジトリは PR が 300 件を超えており、書いた時点で既に上限に張り付いていた)。 **削除はしない。** 出力は人間がそのまま貼れる git push --delete までで、exe は実行 しない (ADR-022 / ADR-028)。ブランチ削除は外部可視かつ着手済みマーカーの破棄でもある。 **出力に wall-clock を含めない。** 同じ状態なら同じ出力にして週次 diff を取れるようにし、 「今週新たに浮いたブランチ」だけを読めるようにした。実行時刻は呼び手が記録する。 - 新規 crate src/cli-stale-branch-scan (classify = 純粋判定 / collect = fail-closed I/O) - unit test 25 件。取得失敗・上限到達・欠損フィールドはすべて Err に倒し、0 件と報告しない - pnpm stale-branch-scan / build:cli-stale-branch-scan (build:all にも登録) - ADR-031 に § 残存ブランチ検出 と L2/L3 の置き場所判断基準を追記 実走: 本リポジトリで削除候補 0 件、open PR 3 本 (#320/#324/#373) を正しく対象外と判定。 weekly-review skill (別リポジトリ claude-code-skills) 側の Step 1.0 / Phase 3 追記は 本 PR に含まれない。編集は済んでいるがコミットは未実施。 --- pre-push review 対応 --- security REJECT (High, SEC-NEW-cli-stale-branch-scan-main-L154): git の ref 名規則は `;` バッククォート `|` `$()` を許す (git check-ref-format で実測)。push 権限を持つ誰か (夜間/cloud harness の自動化を含む) が細工したブランチ名は、本レポートが設計として 出す「そのまま貼れる削除コマンド」経由でコピペ実行時に任意コマンドを実行し得る。 安全文字の allowlist を導入し、外れる名前には削除コマンドを生成しない。 **指摘の修正範囲は狭かったので広げた。** 同じブランチ名は削除提案表の 1 列目と 参考表 2 つにも出るため、コマンド欄だけ塞いでもバッククォート (コードスパン脱出) と `|` (表の列構造破壊) が残る。描画の全経路を単一の branch_cell() へ通し、危険文字を `?` へ潰したうえで印を付ける形にした。出口ごとに個別対策を足すと出口が増えたときに 同じ穴が空くため、安全文字集合の定義は 1 箇所に保つ。 simplicity (SIM-NEW-cli-stale-branch-scan-classify-L283): trunk 名を独自 hardcode して おり lib_jj_helpers::TRUNK_BOOKMARKS からずれて develop/trunk を守れていなかった。 is_trunk_bookmark 呼び出しに変更し、push-runner-config.toml の default_branch も 追加の保護対象として読む。 test 30 件 (危険文字が 3 表いずれにも生で出ないことの回帰固定を含む)。 workspace 全体 2,000 pass / clippy clean / lint 0 error。 --- pre-push review 2 巡目 --- simplicity needs_fix (High, SIM-NEW-cli-stale-branch-scan-main-L121): configured_trunk_branch() が top-level default_branch しか読まないが、本リポジトリの push-runner-config.toml は top-level をコメントアウトし [pr_size_check] / [docs_only_routing] の section override だけで trunk 名を決めている (ADR-051 の cross-config coupling)。master は TRUNK_BOOKMARKS に含まれるため masked だが、標準外の trunk 名を section override だけで設定している リポジトリでは None に落ち、trunk が削除候補として貼れるコマンド付きで出得た。 cli-push-runner の effective_default_branch() と同じ top-level → section fallback へ修正。 **実 config に対する回帰テストを追加した** (--ignored)。既存 5 件は合成 TOML で分岐を 固めるだけで、指摘の起点だった「このリポジトリの実 config が section override 構成で ある」事実を突いていない。実ファイルの構成が変わって解決不能になっても合成テストは 気づけないため、実ファイルから "master" が解決できることを値まで assert する。 test 36 件 (うち 1 件は cwd 依存の --ignored)。workspace 全体 2,005 pass / clippy clean。 --- CodeRabbit レビュー対応 (#377、3 件) --- - ADR-031 に L3 skill の所在を明記。skill は本リポジトリではなく skills repo ($CLAUDE_SKILLS_REPO) にあり ~/.claude/skills/ へ deploy する構成 (ADR-062 の /monthly-review と同じ)。ADR がそれを書いていなかったため「ADR は L3 を定義して いるが実装が無い」と読めていた。各層の実体がどこにあるかの表を追加し、skill 側の 変更は PR diff に現れない帰結も明記した。 - ADR-031 の削除コマンドを実装に合わせて `--delete -- <branch>` へ修正。あわせて 「貼れるコマンドである以上ブランチ名は攻撃面」という設計理由 (-- 区切りと allowlist の 2 つの手当て) を ADR 側にも残した。実装だけが知っている状態を解消。 - gh 自体の失敗にもブランチ名を付ける。map_err が parse_pr_list の結果にしか 掛かっておらず、起動失敗 / timeout / 非ゼロ exit ではどのブランチで止まったか 分からなかった。最大 100 ブランチを順に回すため fail-closed 停止後の切り分けが 効かない。実行層を closure で受ける形にし、失敗経路をネットワーク無しで固定する 回帰テストを 3 件追加。 test 40 件 (うち --ignored 1)。workspace 全体 2,009 pass / clippy clean / lint 0 error。
aloekun
force-pushed
the
feat/stale-branch-scan
branch
from
August 9, 2026 13:19
c0e29c0 to
17a14cd
Compare
Owner
Author
CodeRabbit レビュー対応 (3 件すべて修正)
1. L3 skill の所在について指摘は事実として正しく、原因は ADR-031 が skill の所在を書いていなかったことです。本プロジェクトの skill は本リポジトリではなく skills repo ( ADR-031 に各層の実体がどこにあるかの表を追加しました。あわせて skill 側の変更は本リポジトリの PR diff に現れないという帰結と、L2/L3 をまたぐ変更では両リポジトリの同期を人間が確認する必要がある (自動照合の仕組みは無く 本 PR に対応する skill 側の変更 (Step 1.0 = takt 起動前の 2. 削除コマンドの記法ご指摘のとおり実装は
理由を実装だけが知っている状態を解消しました。 3.
|
aloekun
added a commit
that referenced
this pull request
Aug 10, 2026
あわせて post-merge feedback (#376/#377/#380/#381/#382) の採用分 10 件を順位 402-411 へ 登録した。2026-08-10 に採用候補を系統別へ分類し、ユーザーが採否を決定したもの。 採用: 系統 A (観測の完全性) 3 件 / 系統 B (重複実装の予防) 3 件 / 系統 C (shell・config パースの安全性) 3 件 却下: 系統 D (workflow セキュリティ標準化) / 系統 E (PAT 失効監視) — 様子見 形を変えて採用: 系統 F — 「CLAUDE.md に rustfmt 非適用の方針を書く」ではなく 「cargo fmt を PreToolUse でブロックする」(順位 411) 系統 F の変更理由 (ユーザー判断): 規約は CLAUDE.md に書いた時点で毎セッション読まれ コンテキストを圧迫するが、PreToolUse hook は発火するまでコストがゼロで、ブロックと 同時に正しいコマンドをフィードバックできる。読み手は規約を覚えていなくても正しい経路へ 到達する。ADR-042 の mechanizable 判定を満たすため機構側が正しい。 **この非対称は現行 ADR-042 に無い**。同 ADR の判断基準は「機械判定できるか」「投資対効果」 が中心で、「規約は常時コンテキストを消費し hook は発火時のみ」という観点が明示されて いない。ルール追加を検討するたびに効く一般則なので、順位 411 の作業範囲に ADR-042 への 追記を含めた。
aloekun
added a commit
that referenced
this pull request
Aug 10, 2026
あわせて post-merge feedback (#376/#377/#380/#381/#382) の採用分 10 件を順位 402-411 へ 登録した。2026-08-10 に採用候補を系統別へ分類し、ユーザーが採否を決定したもの。 採用: 系統 A (観測の完全性) 3 件 / 系統 B (重複実装の予防) 3 件 / 系統 C (shell・config パースの安全性) 3 件 却下: 系統 D (workflow セキュリティ標準化) / 系統 E (PAT 失効監視) — 様子見 形を変えて採用: 系統 F — 「CLAUDE.md に rustfmt 非適用の方針を書く」ではなく 「cargo fmt を PreToolUse でブロックする」(順位 411) 系統 F の変更理由 (ユーザー判断): 規約は CLAUDE.md に書いた時点で毎セッション読まれ コンテキストを圧迫するが、PreToolUse hook は発火するまでコストがゼロで、ブロックと 同時に正しいコマンドをフィードバックできる。読み手は規約を覚えていなくても正しい経路へ 到達する。ADR-042 の mechanizable 判定を満たすため機構側が正しい。 **この非対称は現行 ADR-042 に無い**。同 ADR の判断基準は「機械判定できるか」「投資対効果」 が中心で、「規約は常時コンテキストを消費し hook は発火時のみ」という観点が明示されて いない。ルール追加を検討するたびに効く一般則なので、順位 411 の作業範囲に ADR-042 への 追記を含めた。 ## 計画書 (harness-improvement-plan.md) の WP-18 節を再編成 **WP-18 で生んだ問題と、WP-18 の運用で日常的に踏む問題を WP-18 の外へ押し出さない** (2026-08-10 ユーザー方針) ため、残作業を 3 区分へ分けて完了条件を明示した。 従来は観測と派生タスクが 1 表に混在し、WP-18 の完了に何が要るのかが読み取れなかった。 - (1) 観測待ち — 機構は整備済みで事象か期限を待つもの - (2) 運用問題の対処 — WP-18 が生んだ (基準 1) / WP-18 の運用で踏む潜在バグ (基準 2)。 順位 397 / 398-400 / 401 / 410。**完了条件に含める** - (3) WP-18 外の派生 — 順位 396 / 411 / 402-409 等。完了条件に含めない (3) を完了条件から外すのは、§ 7 の退役条件が「全 WP が完了または見送り」である以上、 リポジトリ全体の一般則を WP-18 に紐づけると計画書が永久に退役できなくなるため。 ただし**優先度が低いという意味ではない** — 順位 396 (flaky テスト) と 411 (cargo fmt ブロック) はいずれも高優先度で、WP-18 とは独立に早期着手する旨を明記した。 あわせて古い記述を実測に合わせた: - 見出しの「実装・スモークは 2026-08-08 までにほぼ完了」→ 決定 16 という新規実装が 2026-08-10 に入ったため「観測中 + 運用問題の対処中」へ - 「前 2 者は順位 394 後の run で判定できる」→ 順位 394 は完了済みで実際の前提は決定 16。 同一ファイル内の自己矛盾だった - WP-17 残課題節にも同じ「順位 394 後の run」が残っていたため同期。あわせて 「代替解は draft 廃止」が誤りだったことも記録した ## todo 側 - 順位 396 を Tier 2 → **Tier 1** へ格上げ (ユーザー判断)。単発の Severity では Tier 2 相当だが、flaky テストは「また flake だろう」という読み替えを生み実バグの見落とし 経路になるため。両 OS matrix (ADR-065) の信号品質を守る意味で早期に潰す - 順位 411 に早期着手の根拠を追記 (cargo fmt は反射的に実行されやすい) - **却下を negative result として記録**: 系統 D / E は様子見。trunk 保護の drift 対処 2 件は却下 (予防側は順位 405 で押さえた / 共有 lib 化は network isolation 設計と 抵触しうる)。**再採用条件は「同型の drift が今後も再発する場合」**と明記した
aloekun
added a commit
that referenced
this pull request
Aug 10, 2026
あわせて post-merge feedback (#376/#377/#380/#381/#382) の採用分 10 件を順位 402-411 へ 登録した。2026-08-10 に採用候補を系統別へ分類し、ユーザーが採否を決定したもの。 採用: 系統 A (観測の完全性) 3 件 / 系統 B (重複実装の予防) 3 件 / 系統 C (shell・config パースの安全性) 3 件 却下: 系統 D (workflow セキュリティ標準化) / 系統 E (PAT 失効監視) — 様子見 形を変えて採用: 系統 F — 「CLAUDE.md に rustfmt 非適用の方針を書く」ではなく 「cargo fmt を PreToolUse でブロックする」(順位 411) 系統 F の変更理由 (ユーザー判断): 規約は CLAUDE.md に書いた時点で毎セッション読まれ コンテキストを圧迫するが、PreToolUse hook は発火するまでコストがゼロで、ブロックと 同時に正しいコマンドをフィードバックできる。読み手は規約を覚えていなくても正しい経路へ 到達する。ADR-042 の mechanizable 判定を満たすため機構側が正しい。 **この非対称は現行 ADR-042 に無い**。同 ADR の判断基準は「機械判定できるか」「投資対効果」 が中心で、「規約は常時コンテキストを消費し hook は発火時のみ」という観点が明示されて いない。ルール追加を検討するたびに効く一般則なので、順位 411 の作業範囲に ADR-042 への 追記を含めた。 ## 計画書 (harness-improvement-plan.md) の WP-18 節を再編成 **WP-18 で生んだ問題と、WP-18 の運用で日常的に踏む問題を WP-18 の外へ押し出さない** (2026-08-10 ユーザー方針) ため、残作業を 3 区分へ分けて完了条件を明示した。 従来は観測と派生タスクが 1 表に混在し、WP-18 の完了に何が要るのかが読み取れなかった。 - (1) 観測待ち — 機構は整備済みで事象か期限を待つもの - (2) 運用問題の対処 — WP-18 が生んだ (基準 1) / WP-18 の運用で踏む潜在バグ (基準 2)。 順位 397 / 398-400 / 401 / 410。**完了条件に含める** - (3) WP-18 外の派生 — 順位 396 / 411 / 402-409 等。完了条件に含めない (3) を完了条件から外すのは、§ 7 の退役条件が「全 WP が完了または見送り」である以上、 リポジトリ全体の一般則を WP-18 に紐づけると計画書が永久に退役できなくなるため。 ただし**優先度が低いという意味ではない** — 順位 396 (flaky テスト) と 411 (cargo fmt ブロック) はいずれも高優先度で、WP-18 とは独立に早期着手する旨を明記した。 あわせて古い記述を実測に合わせた: - 見出しの「実装・スモークは 2026-08-08 までにほぼ完了」→ 決定 16 という新規実装が 2026-08-10 に入ったため「観測中 + 運用問題の対処中」へ - 「前 2 者は順位 394 後の run で判定できる」→ 順位 394 は完了済みで実際の前提は決定 16。 同一ファイル内の自己矛盾だった - WP-17 残課題節にも同じ「順位 394 後の run」が残っていたため同期。あわせて 「代替解は draft 廃止」が誤りだったことも記録した ## todo 側 - 順位 396 を Tier 2 → **Tier 1** へ格上げ (ユーザー判断)。単発の Severity では Tier 2 相当だが、flaky テストは「また flake だろう」という読み替えを生み実バグの見落とし 経路になるため。両 OS matrix (ADR-065) の信号品質を守る意味で早期に潰す - 順位 411 に早期着手の根拠を追記 (cargo fmt は反射的に実行されやすい) - **却下を negative result として記録**: 系統 D / E は様子見。trunk 保護の drift 対処 2 件は却下 (予防側は順位 405 で押さえた / 共有 lib 化は network isolation 設計と 抵触しうる)。**再採用条件は「同型の drift が今後も再発する場合」**と明記した --- CodeRabbit レビュー対応 (#384、5 件すべて修正) --- 1. WP-18 完了条件でスモーク未確定の扱いが不明確 (Major) (c) だけを非必須と書き (a)(b) の扱いが無かった。(a)(b) は事象待ちで**自力で発生させ られない**ため、条件に含めると WP を閉じられない。3 件すべてを非ブロッカーとし、 理由と移管先・期限を表で明記した。(a)(b) は 2026-11-06 時点で未観測なら 「機会が来なかった」として見送り ADR-067 の bounded lifetime へ委ねる。 2. 順位 411 の要約が詳細計画と不一致 (Minor) summary は「正しいコマンドを提示」だが、cargo fmt に**代替コマンドは存在しない** (手で直すのが正)。「正しい対処を提示」へ変更し、詳細側にもその旨を明記した。 3. 順位 398 の完了判定を対象 PR に束縛すべき (Major) 「report 生成を完了根拠にする」案が不十分だった。copy_feedback_report は find_latest_run_dir で最新 run を選ぶだけで **pr_number と照合していない**ため、 別 PR の report を現在の PR の {pr_number}.md へコピーし得る。また takt の終了は timeout や失敗でも起こるので終了した事実は report 完成を証明しない。実装を読んで 裏付けたうえで、完了判定には「成功終了」と「対象 PR のものであること」の両方が 要る旨を追記した。本セッションで実際に context.json が別 PR を指していた事象とも 同型である。 4. 旧語彙 lint の extensions から yaml が漏れている (Minor) 拡張子は eq_ignore_ascii_case の文字列一致で **yml と yaml は別物**。本リポジトリは .github/workflows/*.yml と .coderabbit.yaml の両方を持つため、yaml を落とすと 後者が未検査になる。両方を対象に加え、理由も併記した。 5. cargo fmt の検出対象が未定義 (Major) 完全一致だけでは cargo fmt --all / cargo +stable fmt / rustup run stable cargo fmt / cargo-fmt が素通りする。作業計画の先頭に「検出範囲を先に決める」を追加し、完了基準に 「完全一致に限定する場合は素通りする形態を明記する」ことを求める形にした。 いずれも実物 (takt.rs の実装 / linter の拡張子判定 / リポジトリ内の .yml と .yaml の 共存) を確認したうえで妥当と判断している。
aloekun
added a commit
that referenced
this pull request
Aug 10, 2026
あわせて post-merge feedback (#376/#377/#380/#381/#382) の採用分 10 件を順位 402-411 へ 登録した。2026-08-10 に採用候補を系統別へ分類し、ユーザーが採否を決定したもの。 採用: 系統 A (観測の完全性) 3 件 / 系統 B (重複実装の予防) 3 件 / 系統 C (shell・config パースの安全性) 3 件 却下: 系統 D (workflow セキュリティ標準化) / 系統 E (PAT 失効監視) — 様子見 形を変えて採用: 系統 F — 「CLAUDE.md に rustfmt 非適用の方針を書く」ではなく 「cargo fmt を PreToolUse でブロックする」(順位 411) 系統 F の変更理由 (ユーザー判断): 規約は CLAUDE.md に書いた時点で毎セッション読まれ コンテキストを圧迫するが、PreToolUse hook は発火するまでコストがゼロで、ブロックと 同時に正しいコマンドをフィードバックできる。読み手は規約を覚えていなくても正しい経路へ 到達する。ADR-042 の mechanizable 判定を満たすため機構側が正しい。 **この非対称は現行 ADR-042 に無い**。同 ADR の判断基準は「機械判定できるか」「投資対効果」 が中心で、「規約は常時コンテキストを消費し hook は発火時のみ」という観点が明示されて いない。ルール追加を検討するたびに効く一般則なので、順位 411 の作業範囲に ADR-042 への 追記を含めた。 ## 計画書 (harness-improvement-plan.md) の WP-18 節を再編成 **WP-18 で生んだ問題と、WP-18 の運用で日常的に踏む問題を WP-18 の外へ押し出さない** (2026-08-10 ユーザー方針) ため、残作業を 3 区分へ分けて完了条件を明示した。 従来は観測と派生タスクが 1 表に混在し、WP-18 の完了に何が要るのかが読み取れなかった。 - (1) 観測待ち — 機構は整備済みで事象か期限を待つもの - (2) 運用問題の対処 — WP-18 が生んだ (基準 1) / WP-18 の運用で踏む潜在バグ (基準 2)。 順位 397 / 398-400 / 401 / 410。**完了条件に含める** - (3) WP-18 外の派生 — 順位 396 / 411 / 402-409 等。完了条件に含めない (3) を完了条件から外すのは、§ 7 の退役条件が「全 WP が完了または見送り」である以上、 リポジトリ全体の一般則を WP-18 に紐づけると計画書が永久に退役できなくなるため。 ただし**優先度が低いという意味ではない** — 順位 396 (flaky テスト) と 411 (cargo fmt ブロック) はいずれも高優先度で、WP-18 とは独立に早期着手する旨を明記した。 あわせて古い記述を実測に合わせた: - 見出しの「実装・スモークは 2026-08-08 までにほぼ完了」→ 決定 16 という新規実装が 2026-08-10 に入ったため「観測中 + 運用問題の対処中」へ - 「前 2 者は順位 394 後の run で判定できる」→ 順位 394 は完了済みで実際の前提は決定 16。 同一ファイル内の自己矛盾だった - WP-17 残課題節にも同じ「順位 394 後の run」が残っていたため同期。あわせて 「代替解は draft 廃止」が誤りだったことも記録した ## todo 側 - 順位 396 を Tier 2 → **Tier 1** へ格上げ (ユーザー判断)。単発の Severity では Tier 2 相当だが、flaky テストは「また flake だろう」という読み替えを生み実バグの見落とし 経路になるため。両 OS matrix (ADR-065) の信号品質を守る意味で早期に潰す - 順位 411 に早期着手の根拠を追記 (cargo fmt は反射的に実行されやすい) - **却下を negative result として記録**: 系統 D / E は様子見。trunk 保護の drift 対処 2 件は却下 (予防側は順位 405 で押さえた / 共有 lib 化は network isolation 設計と 抵触しうる)。**再採用条件は「同型の drift が今後も再発する場合」**と明記した --- CodeRabbit レビュー対応 (#384、5 件すべて修正) --- 1. WP-18 完了条件でスモーク未確定の扱いが不明確 (Major) (c) だけを非必須と書き (a)(b) の扱いが無かった。(a)(b) は事象待ちで**自力で発生させ られない**ため、条件に含めると WP を閉じられない。3 件すべてを非ブロッカーとし、 理由と移管先・期限を表で明記した。(a)(b) は 2026-11-06 時点で未観測なら 「機会が来なかった」として見送り ADR-067 の bounded lifetime へ委ねる。 2. 順位 411 の要約が詳細計画と不一致 (Minor) summary は「正しいコマンドを提示」だが、cargo fmt に**代替コマンドは存在しない** (手で直すのが正)。「正しい対処を提示」へ変更し、詳細側にもその旨を明記した。 3. 順位 398 の完了判定を対象 PR に束縛すべき (Major) 「report 生成を完了根拠にする」案が不十分だった。copy_feedback_report は find_latest_run_dir で最新 run を選ぶだけで **pr_number と照合していない**ため、 別 PR の report を現在の PR の {pr_number}.md へコピーし得る。また takt の終了は timeout や失敗でも起こるので終了した事実は report 完成を証明しない。実装を読んで 裏付けたうえで、完了判定には「成功終了」と「対象 PR のものであること」の両方が 要る旨を追記した。本セッションで実際に context.json が別 PR を指していた事象とも 同型である。 4. 旧語彙 lint の extensions から yaml が漏れている (Minor) 拡張子は eq_ignore_ascii_case の文字列一致で **yml と yaml は別物**。本リポジトリは .github/workflows/*.yml と .coderabbit.yaml の両方を持つため、yaml を落とすと 後者が未検査になる。両方を対象に加え、理由も併記した。 5. cargo fmt の検出対象が未定義 (Major) 完全一致だけでは cargo fmt --all / cargo +stable fmt / rustup run stable cargo fmt / cargo-fmt が素通りする。作業計画の先頭に「検出範囲を先に決める」を追加し、完了基準に 「完全一致に限定する場合は素通りする形態を明記する」ことを求める形にした。 いずれも実物 (takt.rs の実装 / linter の拡張子判定 / リポジトリ内の .yml と .yaml の 共存) を確認したうえで妥当と判断している。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cli-stale-branch-scanを追加 (pnpm stale-branch-scan)。todo 順位 395git push origin --delete -- <branch>であって、exe も skill も実行しない (ADR-022 / ADR-028)--ignored)。取得失敗・上限到達・欠損フィールドはすべてErrに倒し、0 件と報告しないContext
Why: クローズ済み PR #365 のブランチを手で消したことで ADR-072 決定 3 の除外マーカーが失われ、同じ順位が再選択された (PR #373)。決定 3 自体は設計どおりで、ブランチの存在が着手済みマーカーである以上、浮いたブランチを定期的に片付ける場が要る。
設計判断 — takt workflow には置けなかった: todo は「facet 相乗り or workflow 内の決定論 scan」を想定していたが、weekly-review.yaml は全 provider に
network_access: falseを課している (他 3 パイプラインはtrue)。検出にはgit ls-remote/ghが要るため、1 つの scan のためにフラグを反転すると whole-tree review 6 facet すべての隔離が同時に緩む。/monthly-reviewがcli-telemetry-reportを呼ぶのと同じ ADR-031 L3 (skill 側の決定論層) に置いた。判定規則: 紐づく PR が全て closed/merged → 削除候補 / open が 1 本でもある → 対象外 (close 後の再オープンが実在する) / PR が 1 件も無い → 対象外 (PR 未作成の作業中ブランチと区別できない) / trunk は常に対象外 /
claude/nightly-*は除外しない (除外するとその順位が二度と選ばれない) / state が未知の値 → open 扱い (保護側)。実装中に設計が破綻した点: 当初は PR を全件取得していたが、実走で即 fail-closed が発火した — 本リポジトリは PR が 300 件を超えており、書いた時点で既に上限に張り付いていた。上限を上げるのは対症療法 (総 PR 数は単調増加) なので、入力領域をブランチ単位に変更した (
--head <branch>で 1 本ずつ)。remote ブランチ数は運用上小さく有界。Scope decision: 1 PR。exe 単体では weekly-review から呼ばれず、ADR/todo/pnpm 配線なしでは機能しない。PR diff 1,128 行で warning 閾値 800 超・block 閾値 1500 未満。
Validation
cargo test --workspace: 2,006 pass / 0 fail。本 crate は 36 pass +--ignored1 passcargo clippy --workspace --all-targets -- -D warnings: cleanpnpm lint:md(127 files) /pnpm lint:docs: 0 errorpnpm pushpre-push review: simplicity / security とも approved (3 iterations)git check-ref-formatで;`|$()が ref 名に使えることを確認 → allowlist + 全描画経路の無害化--forceが有効な ref 名であることを確認 → 削除コマンドに--区切りを挿入develop/trunkが未保護だったことを確認 →lib_jj_helpers::is_trunk_bookmarkへ寄せるlib_subprocess::wait_with_timeout_basicで 60s/weekly-review起動時に Step 1.0 が呼ばれることを確認するReferences
--ignored運用) / ADR-051 (config 解決のミラー)Summary by CodeRabbit
新機能
ドキュメント