feat: weekly last-run をメイン workspace に canonical 化 (ADR-045 / PR-N2) - #300
Merged
Conversation
… / PR-N2) secondary jj workspace から canonical な (メイン) workspace root を解決する fs ヘルパーを追加。 gitignore 済み untracked 状態ファイル (weekly-review-last-run.json 等) は per-checkout で materialize され secondary workspace には存在しない問題への基盤 (ADR-045)。 - .jj/repo がディレクトリ → colocated main → 自身を返す - .jj/repo がファイル → 内容の main store パス (相対は .jj/ 基準) の 2 階層上がメイン root - .jj 不在 / 読取失敗 / 導出パス不存在 → None (caller は現 root に fail-open) resolve_git_dir と同じ layout 解釈 (相対基準・verbatim prefix 剥がし) を共有。テストは fixture + 実 jj E2E (ignored) を resolve_git_dir パターンで流用。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l 化 (ADR-045 / PR-N2) gitignore 済み untracked の weekly-review-last-run.json は workspace ローカルで secondary workspace に存在せず、メイン側では常に「未実行」判定で reminder が永久発火していた (2026-07-19 実観測、ADR-045 状態分裂)。 - hooks-session-start に lib-jj-helpers 依存を追加 - compute_weekly_review_reminder_nudge の last-run 読込を resolve_main_workspace_root(cwd).unwrap_or(cwd) 基準に変更 (導出不能は現 root に fail-open) - failed marker / pending JSON はレビュー成果物として workspace ローカルのまま維持 (線引きを doc 明記) - doc comment の「last_run_at は workspace 不変」誤記を訂正 (値は不変だがファイル所在は workspace 依存) secondary レイアウト (.jj/repo ファイル) でメイン root の last-run を読みつつ failed marker は 現 workspace から読むことを検証する unit test を追加。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
weekly-review last-run が secondary workspace 側にのみ存在しメイン側で永久「未実行」発火した silent bug (2026-07-19 実観測) を ADR に定着。 - ADR-031 § トリガー方式と reminder: last-run のメイン workspace canonical 化の決定を追記。 「last_run_at は workspace 不変」誤記の是正 (値は checkout 不変だがファイル所在は workspace 依存)、 resolve_main_workspace_root による読込 canonical 化、failed marker/pending JSON は per-workspace 維持。 - ADR-045: gitignore 済み untracked 状態ファイルの workspace 分裂を silent bug class として新設。 mtime リセット (CR #233) と対になる実例として対比表を追加し、per-workspace/global の判定基準を明文化。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 分割コミット粒度・skills repo は内容編集のみ (commit/deploy は skills 側に委任)・ 検証結果 (cargo test --workspace green / hooks-session-start 93 passed / lib-jj-helpers 37+E2E 2 / clippy clean / build:all 成功 / デプロイ exe を secondary レイアウトで駆動しメイン root の last-run を読んで「18 日経過」を end-to-end 確認、対照の未実行 fail-open も確認)・ 残タスク (運用コピー・削除条件 3 = 新セッション目視・skills deploy) を追記。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthrough週次レビューの Changes週次レビュー状態の canonical 化
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SessionStartHook
participant MainWorkspace
participant ExecutionWorkspace
SessionStartHook->>MainWorkspace: weekly-review-last-run.json を読む
MainWorkspace-->>SessionStartHook: last_run_at を返す
SessionStartHook->>ExecutionWorkspace: failed marker / pending JSON を列挙する
ExecutionWorkspace-->>SessionStartHook: 実行 workspace の成果物を返す
SessionStartHook-->>SessionStartHook: reminder と systemMessage を生成する
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)(該当なし) Applicable Findings (Medium 以下)(該当なし) Filtered (not applicable)(該当なし — レビュー指摘自体がまだ無い) diff 概要 (軽量サマリー)
次のアクション
|
This was referenced Jul 19, 2026
aloekun
added a commit
that referenced
this pull request
Jul 19, 2026
* docs(todo): PR-N1〜N3 post-merge feedback の採用候補を todo 登録 (#288 昇格 + 329-332 新設) PR #299/#300/#301 の post-merge feedback から採用候補 5 件を系統別に登録: - #288 昇格 (系統 A): pre-push review の diff スコープ漏れが 3 連続再発 (Severity High)。 post-merge 全 run 集約に加え push-runner [diff] stage の tip-only 範囲修正を統合し Tier2→Tier1 - 329 (系統 B): 新規 ADR 起案時の「判断根拠 × 既存 ADR 定義」矛盾チェックリスト - 330 (系統 B): 行動要求 nudge の 2 チャネル返却 + 多義的戻り値 struct 化 convention - 331 (系統 C): systemMessage 含む JSON 出力の exe-spawn E2E テスト - 332 (系統 D): pnpm build:all の Windows cp.exe PATH 自動化 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(todo): PR #302 CodeRabbit 指摘を反映 (#288 fail-closed 祖先検証 / schema 移行 / Windows 検出 / ADR 意図変更) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aloekun
added a commit
that referenced
this pull request
Jul 21, 2026
## 問題 `[diff] command` に `jj diff -r @` が直書きされており、AI レビュアーには **tip コミットの diff しか渡っていなかった**。祖先コミットは pre-push のセルフレビューを 一度も経ずに merge される。 同じパイプライン内で `pr_size_check` と `docs_only_routing` は `<base>..@` (PR 範囲) を見ており、`[diff]` だけが非対称だった。実際 PR #311 では 695 行の PR に対して 37 行だけがレビュー対象になり、レビュアーは渡された 37 行を見て正しく「docs-only」と 判定していた。**レビュアー側からは「渡された diff が PR 全体か」を検証できない**ため、 この誤りは誰にも検知されない。 docs/todo-summary2.md 順位 288 として既知で、Severity High で PR #268/#300/#301 に 続き #311 が 4 回目の再発。 ## 変更 - **範囲の真実源を 1 箇所に**: top-level `default_branch` を新設し、`diff` / `docs_only_routing` / `pr_size_check` の 3 stage が `Config::resolve_base_branch` 経由で同じ値を使う。従来は section ごとに独立した `default_branch` を持ち、 「値を同期する義務」を config コメントで課していた (docs_only_routing.rs の doc に 明記されていた) が、義務はコード上の不変条件ではないため非対称を許していた。 section 側は後方互換の override として残す (派生プロジェクトの既存 config 対策)。 - **config から revset を排除**: `[diff] command` は `{{PR_RANGE}}` プレースホルダを 使う。push-runner が `<base>..@` に展開するため、狭い範囲を書く余地が無くなる。 - **範囲カバレッジ検査 (fail-closed)**: 生成した diff が PR 範囲の全変更ファイルを 含むか `jj diff --summary` と突き合わせ、不足があれば exit 5 で中断する。config の 書き方に依存せず「レビュー範囲 < PR 範囲」を検知できるため、未更新の派生プロジェクト config も捕まえる。summary 取得失敗・`diff --git` ヘッダ不在 (= 収録ファイルを特定 できない) も「網羅している」に倒さずエラーにする (ADR-043)。 - **`--git` 形式へ切替 (順位 264)**: 範囲検査がヘッダを読む要件に加え、jj 既定形式は 色を落とすと `+`/`-` が消えて LLM レビュアーが削除を追加と誤読する (PR #256 で simplicity-review が todo 25 行の削除を追加と誤読し false positive REJECT、約 19 分 浪費)。 - `templates/push-runner-config.toml` も同時修正 (deploy:hooks で派生配布されるため)。 ## 実測で見つけた副次バグ 範囲 summary の取得を当初 shell 経由 (`[diff] command` と同じ経路) で実装したが、 実シェルで叩いたところ **cmd.exe はクォートを除去せず jj に渡す**ため `Revision '"<base>..@"' doesn't exist` で必ず失敗した。sh は除去するので Linux だけ 通る = Windows の全 push が fail-closed で止まる形。ユニットテストは summary 取得を 注入していたため検出できなかった。direct args 呼び出し (docs_only_routing の既存 `run_jj_diff_summary`) を共有する形に修正し、doc に理由を残した。両 stage は 「PR 範囲の変更ファイル一覧」という同一の問いを扱うため、別実装にすると本 PR が 排除した非対称を再導入することになる。 ## 検証 cargo test --workspace 全 pass / clippy clean。範囲検査は incident 形状 (PR 2 ファイル 変更に対し diff は tip の 1 ファイルのみ) の再現テストで固定し、fail-closed 経路 (summary 取得失敗 / ヘッダ不在) と過剰検知しない経路 (空 PR 範囲 / Windows パス 区切りの正規化) も併せて固定した。base branch 解決は 3 stage が同一範囲に解決される ことと override の優先順位を machine-enforce している。
aloekun
added a commit
that referenced
this pull request
Jul 21, 2026
) * fix(push-runner): AI レビュー対象 diff を PR 全体に修正し範囲を機械検査する (順位 288/264) ## 問題 `[diff] command` に `jj diff -r @` が直書きされており、AI レビュアーには **tip コミットの diff しか渡っていなかった**。祖先コミットは pre-push のセルフレビューを 一度も経ずに merge される。 同じパイプライン内で `pr_size_check` と `docs_only_routing` は `<base>..@` (PR 範囲) を見ており、`[diff]` だけが非対称だった。実際 PR #311 では 695 行の PR に対して 37 行だけがレビュー対象になり、レビュアーは渡された 37 行を見て正しく「docs-only」と 判定していた。**レビュアー側からは「渡された diff が PR 全体か」を検証できない**ため、 この誤りは誰にも検知されない。 docs/todo-summary2.md 順位 288 として既知で、Severity High で PR #268/#300/#301 に 続き #311 が 4 回目の再発。 ## 変更 - **範囲の真実源を 1 箇所に**: top-level `default_branch` を新設し、`diff` / `docs_only_routing` / `pr_size_check` の 3 stage が `Config::resolve_base_branch` 経由で同じ値を使う。従来は section ごとに独立した `default_branch` を持ち、 「値を同期する義務」を config コメントで課していた (docs_only_routing.rs の doc に 明記されていた) が、義務はコード上の不変条件ではないため非対称を許していた。 section 側は後方互換の override として残す (派生プロジェクトの既存 config 対策)。 - **config から revset を排除**: `[diff] command` は `{{PR_RANGE}}` プレースホルダを 使う。push-runner が `<base>..@` に展開するため、狭い範囲を書く余地が無くなる。 - **範囲カバレッジ検査 (fail-closed)**: 生成した diff が PR 範囲の全変更ファイルを 含むか `jj diff --summary` と突き合わせ、不足があれば exit 5 で中断する。config の 書き方に依存せず「レビュー範囲 < PR 範囲」を検知できるため、未更新の派生プロジェクト config も捕まえる。summary 取得失敗・`diff --git` ヘッダ不在 (= 収録ファイルを特定 できない) も「網羅している」に倒さずエラーにする (ADR-043)。 - **`--git` 形式へ切替 (順位 264)**: 範囲検査がヘッダを読む要件に加え、jj 既定形式は 色を落とすと `+`/`-` が消えて LLM レビュアーが削除を追加と誤読する (PR #256 で simplicity-review が todo 25 行の削除を追加と誤読し false positive REJECT、約 19 分 浪費)。 - `templates/push-runner-config.toml` も同時修正 (deploy:hooks で派生配布されるため)。 ## 実測で見つけた副次バグ 範囲 summary の取得を当初 shell 経由 (`[diff] command` と同じ経路) で実装したが、 実シェルで叩いたところ **cmd.exe はクォートを除去せず jj に渡す**ため `Revision '"<base>..@"' doesn't exist` で必ず失敗した。sh は除去するので Linux だけ 通る = Windows の全 push が fail-closed で止まる形。ユニットテストは summary 取得を 注入していたため検出できなかった。direct args 呼び出し (docs_only_routing の既存 `run_jj_diff_summary`) を共有する形に修正し、doc に理由を残した。両 stage は 「PR 範囲の変更ファイル一覧」という同一の問いを扱うため、別実装にすると本 PR が 排除した非対称を再導入することになる。 ## 検証 cargo test --workspace 全 pass / clippy clean。範囲検査は incident 形状 (PR 2 ファイル 変更に対し diff は tip の 1 ファイルのみ) の再現テストで固定し、fail-closed 経路 (summary 取得失敗 / ヘッダ不在) と過剰検知しない経路 (空 PR 範囲 / Windows パス 区切りの正規化) も併せて固定した。base branch 解決は 3 stage が同一範囲に解決される ことと override の優先順位を machine-enforce している。 * docs: ADR-027 に「diff 局所 = 観点の限定であって範囲の限定ではない」を明記 (順位 288/264) ## ADR-027 amendment ADR-027 が狭めたのは reviewer が使う criteria (cross-file 探索を要求しない) であり、レビュー対象に含めるコミットの範囲ではなかった。この 2 つが混同され、 `[diff] command` が tip コミット限定のまま運用されて 4 回の再発を招いたため、 射程を明文化した。 「レビュー対象は PR 範囲全体」と決定した根拠も併記: - 速度は理由にならない。同一 PR でレビュー対象を 37 行 → 1011 行 (27 倍) に 広げても 4m32s → 4m43s の +11 秒。ADR-027 の速度改善は arch-review facet の 除去 (219-270s/iter) によるもので、範囲縮小は寄与していなかった。 - 範囲が狭いことによる見落としはレビュアー側から検知できない (渡された diff が PR 全体かを検証する手段が無い)。 - CodeRabbit backstop はセルフレビューを省く理由にならない。独立した層として併用する。 ## todo 更新 - 順位 264 (`--git` 切替): 完了につきエントリと table 行を削除。 - 順位 288: `[diff]` 範囲修正の部分のみ完了として記録。**残タスク** (post-merge feedback の全 run 集約、bookmark_check.rs の祖先未レビュー穴の検証) は明示して 残す。エントリ全体を消すと未着手部分が失われるため削除しない。 * fix(push-runner): pre-push 範囲検査の欠陥修正 + CodeRabbit 指摘5件対応 (#313)
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
resolve_main_workspace_rootを lib-jj-helpers に追加し、secondary jj workspace から canonical なメイン workspace root を導出できるようにしたlast_run_atは workspace 不変」(weekly_review.rs) と残存「mtime」記述 (hooks_config.rs、CR fix(hooks-session-start): weekly-review staleness を last_run_at ベース化 #233 drift)Context
Why: weekly-review reminder (ADR-031) が secondary workspace (ccht-improve) 側で last-run を書き、メイン workspace には状態ファイルが存在しないため、メイン側セッションで常に「未実行」判定となり reminder が約4週間 silent に発火し続けた (2026-07-19 実観測)。gitignore 済み untracked 状態ファイルが workspace ローカルである盲点で、CR #233 が対処した mtime リセットと対になる silent bug class。
Trigger: weekly-review 通知可視化改善計画 (PR-N1〜N3) の PR-N2。PR-N1 (systemMessage 可視化、#299) に続く直列投入。
Scope decision: 4 コミットに分割 (lib 追加 / session-start 配線 / ADR / 計画書)。skills repo の SKILL.md 変更は同計画に含むが別 git repo のため本 PR には含めず、内容編集のみ実施 (commit/deploy は skills repo 側ライフサイクルに委任)。
Validation
cargo test --workspace: 全 crate green (hooks-session-start 93 pass、lib-jj-helpers 37 pass + 実 jj E2E 2 pass)cargo clippy --workspace --all-targets -- -D warnings: クリーンpnpm pushquality_gate: lint / test / build / rust-lint-test 全 PASSpnpm pushpre-push review: verdict=APPROVE (simplicity + security、2026-07-19).jj/repo→ メイン store) で駆動 → メイン root の last-run (2026-07-01) を読み additionalContext・systemMessage に「18 日経過」を出力。対照の.jj/repo無しは現 root に fail-open して「未実行」References
Summary by CodeRabbit
改善
ドキュメント