fix(merge-pipeline): 夜間 PR の remote 専用ブックマークを検出し --pr を追加 (順位 397) - #385
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:
📝 WalkthroughWalkthrough
Changesmerge pipeline PR検出
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Pipeline
participant Github
participant JJHelpers
participant gh
CLI->>Pipeline: run_pipeline(pr_override)
alt --pr指定あり
Pipeline->>Pipeline: PR番号を使用
else --pr指定なし
Pipeline->>Github: detect_pr_number()
Github->>gh: gh pr view
Github->>JJHelpers: bookmark検索
JJHelpers-->>Github: ローカルまたはリモートbookmark
Github->>gh: bookmarkのPRを検索
gh-->>Github: PR番号
Github-->>Pipeline: 検出結果
end
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)該当なし（findings 未生成） 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: 5
🧹 Nitpick comments (1)
src/lib-jj-helpers/src/bookmarks.rs (1)
503-601: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff統合テストが cwd を変更します。実行条件の逸脱に対する防御を検討してください。
enterはstd::env::set_current_dirでプロセス全体の cwd を変更します。CwdRestoreのDropでパニック時も復元されるため、単独実行では安全です。ただし--test-threads=1を付けずに--ignoredを実行すると、他テストと cwd を共有して不定な失敗になります。#[ignore]の文言で運用を指示している点は妥当です。より堅くするには、query_bookmarks_at/query_remote_bookmarks_atに作業ディレクトリを渡せる内部関数を追加し、cwd 変更を不要にする方法があります。現状は許容範囲です。将来 cwd 依存のテストが増える場合に検討してください。
🤖 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/lib-jj-helpers/src/bookmarks.rs` around lines 503 - 601, No code change is required; the cwd restoration via CwdRestore already makes this ignored integration test safe when run alone. If strengthening it, update query_bookmarks_at and query_remote_bookmarks_at through internal directory-aware helpers so remote_jj::remote_only_bookmark_is_found_via_fallback can pass clone explicitly without changing the process-wide cwd.
🤖 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-013-merge-pipeline.md`:
- Around line 50-51: Update the PR detection documentation to match the current
three-stage bookmark search order (@, `@-`, `@--`), excluding gh pr view from the
flow. In docs/adr/adr-013-merge-pipeline.md lines 50-51, remove or correct the
outdated gh pr list-based description; update docs/adr/adr-013-merge-pipeline.md
line 63 and docs/harness-improvement-plan.md line 191 from “2 stages” to “3
stages,” ensuring all descriptions include `@-`.
In `@src/cli-merge-pipeline/src/main.rs`:
- Around line 37-50: parse_pr_flag で解析した PR 番号が 0 の場合を無効入力として拒否し、既存の引数エラー形式で Err
を返してください。通常の正の PR 番号の処理は維持し、CLI の --pr 0 と --feedback-only 0 が終了コード 2
になるテストを追加してください。
In `@src/lib-jj-helpers/src/lib.rs`:
- Around line 20-22: lib.rs のクレートドキュメントにある再エクスポート説明を、全モジュールではなく bookmarks と
workspace の公開 API のみがクレート直下へ再エクスポートされる内容に修正してください。pipeline_lock の API
が直下から利用できると誤解されない表現にし、既存のパス例は対象範囲に合わせて維持または調整してください。
In `@src/lib-jj-helpers/src/workspace.rs`:
- Around line 118-124: Update strip_windows_verbatim_prefix to handle the UNC
verbatim form \\?\UNC\server\share\... before the generic prefix removal,
converting it to the valid UNC path \\server\share\.... Preserve the existing
behavior for non-UNC verbatim paths and paths without the prefix.
- Around line 87-104: Update resolve_main_workspace_root so the colocated
.jj/repo-directory branch canonicalizes workspace_root before returning it,
while preserving the existing Option<PathBuf> return type. Use the same
strip_windows_verbatim_prefix handling as the file-based main_root branch, and
leave canonicalization failure as None so the caller’s existing fallback remains
responsible for recovery.
---
Nitpick comments:
In `@src/lib-jj-helpers/src/bookmarks.rs`:
- Around line 503-601: No code change is required; the cwd restoration via
CwdRestore already makes this ignored integration test safe when run alone. If
strengthening it, update query_bookmarks_at and query_remote_bookmarks_at
through internal directory-aware helpers so
remote_jj::remote_only_bookmark_is_found_via_fallback can pass clone explicitly
without changing the process-wide cwd.
🪄 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: c2048373-1e93-4ec6-a1b4-b233439213d8
📒 Files selected for processing (11)
docs/adr/adr-013-merge-pipeline.mddocs/adr/adr-024-shared-jj-helpers-library.mddocs/harness-improvement-plan.mddocs/todo-summary2.mddocs/todo21.mdsrc/cli-merge-pipeline/src/github.rssrc/cli-merge-pipeline/src/main.rssrc/cli-merge-pipeline/src/pipeline.rssrc/lib-jj-helpers/src/bookmarks.rssrc/lib-jj-helpers/src/lib.rssrc/lib-jj-helpers/src/workspace.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)
次のアクション
|
aloekun
force-pushed
the
fix/merge-pr-remote-bookmark
branch
from
August 10, 2026 16:09
f4bd203 to
c9f0151
Compare
Contributor
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし Applicable Findings (Medium 以下)該当なし（前回報告の 4 件はすべて解消— 上記参照） Filtered (not applicable)
次のアクション
|
bot が作った PR を人間がマージする経路 (ADR-072 夜間ループ) では PR の head が remote 専用 bookmark しか持たず、ローカル bookmark だけを見ていた PR 検出が空振り していた (#381 のマージで実測)。gh pr merge は ADR-013 の guard でブロックされる ため、ブロックされる経路と動かない経路しかない状態になっていた。 - PR 検出をローカル -> リモート追跡 bookmark の 2 段にする (順位 397 対処案 a) - 失敗時に --pr / bookmark 確認 / jj edit の実行可能な手順を出す (対処案 b) - bookmark 非依存の --pr <番号> を追加する (対処案 c) - lib-jj-helpers を bookmarks.rs / workspace.rs へ分割 (800 行ガイドライン) ローカルを全 revset 走査してからリモートへ移る二段構成にしてあり、ローカル bookmark が見つかる状況では従来と結果が一致するため、共有ヘルパーを使う push-runner / pr-monitor への回帰は起きない。 順位 397 / ADR-013 / ADR-024
aloekun
force-pushed
the
fix/merge-pr-remote-bookmark
branch
from
August 10, 2026 16:21
c9f0151 to
70eaa70
Compare
This was referenced Aug 10, 2026
aloekun
added a commit
that referenced
this pull request
Aug 11, 2026
本セッションで実施した WP-18 (2) 運用問題 5 件の対処 (#385/#386/#388/#389) について、 実走観測の記録・計画書の整理・feedback 採否の登録をまとめて行う。 ## 実走観測の記録 - ADR-072 へ定常運用 2 巡目 (PR #387) の観測を追加する。決定 15-17 投入後の 9 項目が設計どおり動いたことと、review-request の成功判定が「反応の有無」で 止まっている (拒否も success になる) ことを事実として記録する - ADR-019 へレート制限の競合が記録の翌日に実地で再現したことを追加する - 順位 386 の観測を 7 回 → 9 回へ更新する。うち 1 件は空コミットではなく 「近い revset を優先する規則」そのものが原因で、本命の対処案だけでは 解決しない可能性がある点を併記する ## 計画書の整理 (ADR-073 新設) - 完了条件の切り方 (残作業を 3 区分に分け、その WP が生んだ問題は完了条件に 含め、WP 外の派生は含めない) を ADR-073 として切り出す - WP-18 節を 71 行 → 35 行へ整理し、完了記録を削除して残作業のみにする - ローカル実行時の jj workspace 注記を ADR-072 へ移す ## post-merge feedback 採否 (順位 414-432) - 採用候補 24 件のうち 6 件は当該 PR 内で実装済みのため対象外とした (実物と照合して確認) - 採用 19 件を系統 A-G + セッション由来へ分類して登録する - SIGPIPE resilience は却下する。レポートが「実測証拠」とした recovery が 実際には発生しておらず (run 1 回・completed・marker の痕跡なし)、提案内容も ADR-030 §L1 で実装済みだった。feedback レポート自身が根拠を誤った初の実例 として記録し、順位 403 の対象へ含めるよう申し送る ## 付随 - todo21.md が 57KB (50KB 閾値超過) のため todo22.md を新設する - todo-summary.md の「現行の追加先」が todo14.md のまま stale だったので直す - cli-docs-lint が検出した preamble の数詞ずれ (23 → 24) を 9 ファイルで更新する ADR-073 / 順位 414-432
aloekun
added a commit
that referenced
this pull request
Aug 11, 2026
本セッションで実施した WP-18 (2) 運用問題 5 件の対処 (#385/#386/#388/#389) について、 実走観測の記録・計画書の整理・feedback 採否の登録をまとめて行う。 ## 実走観測の記録 - ADR-072 へ定常運用 2 巡目 (PR #387) の観測を追加する。決定 15-17 投入後の 9 項目が設計どおり動いたことと、review-request の成功判定が「反応の有無」で 止まっている (拒否も success になる) ことを事実として記録する - ADR-019 へレート制限の競合が記録の翌日に実地で再現したことを追加する - 順位 386 の観測を 7 回 → 9 回へ更新する。うち 1 件は空コミットではなく 「近い revset を優先する規則」そのものが原因で、本命の対処案だけでは 解決しない可能性がある点を併記する ## 計画書の整理 (ADR-073 新設) - 完了条件の切り方 (残作業を 3 区分に分け、その WP が生んだ問題は完了条件に 含め、WP 外の派生は含めない) を ADR-073 として切り出す - WP-18 節を 71 行 → 35 行へ整理し、完了記録を削除して残作業のみにする - ローカル実行時の jj workspace 注記を ADR-072 へ移す ## post-merge feedback 採否 (順位 414-432) - 採用候補 24 件のうち 6 件は当該 PR 内で実装済みのため対象外とした (実物と照合して確認) - 採用 19 件を系統 A-G + セッション由来へ分類して登録する - SIGPIPE resilience は却下する。レポートが「実測証拠」とした recovery が 実際には発生しておらず (run 1 回・completed・marker の痕跡なし)、提案内容も ADR-030 §L1 で実装済みだった。feedback レポート自身が根拠を誤った初の実例 として記録し、順位 403 の対象へ含めるよう申し送る ## 付随 - todo21.md が 57KB (50KB 閾値超過) のため todo22.md を新設する - todo-summary.md の「現行の追加先」が todo14.md のまま stale だったので直す - cli-docs-lint が検出した preamble の数詞ずれ (23 → 24) を 9 ファイルで更新する ADR-073 / 順位 414-432
aloekun
added a commit
that referenced
this pull request
Aug 11, 2026
本セッションで実施した WP-18 (2) 運用問題 5 件の対処 (#385/#386/#388/#389) について、 実走観測の記録・計画書の整理・feedback 採否の登録をまとめて行う。 ## 実走観測の記録 - ADR-072 へ定常運用 2 巡目 (PR #387) の観測を追加する。決定 15-17 投入後の 9 項目が設計どおり動いたことと、review-request の成功判定が「反応の有無」で 止まっている (拒否も success になる) ことを事実として記録する - ADR-019 へレート制限の競合が記録の翌日に実地で再現したことを追加する - 順位 386 の観測を 7 回 → 9 回へ更新する。うち 1 件は空コミットではなく 「近い revset を優先する規則」そのものが原因で、本命の対処案だけでは 解決しない可能性がある点を併記する ## 計画書の整理 (ADR-073 新設) - 完了条件の切り方 (残作業を 3 区分に分け、その WP が生んだ問題は完了条件に 含め、WP 外の派生は含めない) を ADR-073 として切り出す - WP-18 節を 71 行 → 35 行へ整理し、完了記録を削除して残作業のみにする - ローカル実行時の jj workspace 注記を ADR-072 へ移す ## post-merge feedback 採否 (順位 414-432) - 採用候補 24 件のうち 6 件は当該 PR 内で実装済みのため対象外とした (実物と照合して確認) - 採用 19 件を系統 A-G + セッション由来へ分類して登録する - SIGPIPE resilience は却下する。レポートが「実測証拠」とした recovery が 実際には発生しておらず (run 1 回・completed・marker の痕跡なし)、提案内容も ADR-030 §L1 で実装済みだった。feedback レポート自身が根拠を誤った初の実例 として記録し、順位 403 の対象へ含めるよう申し送る ## 付随 - todo21.md が 57KB (50KB 閾値超過) のため todo22.md を新設する - todo-summary.md の「現行の追加先」が todo14.md のまま stale だったので直す - cli-docs-lint が検出した preamble の数詞ずれ (23 → 24) を 9 ファイルで更新する ADR-073 / 順位 414-432
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
pnpm merge-prの PR 検出を ローカル bookmark → リモート追跡 bookmark の 2 段にし、jj bookmark track無しで夜間 PR をマージできるようにした (順位 397 対処案 a)--pr/jj bookmark list --all-remotes/jj edit) を出力するようにした (対処案 b)--pr <PR番号>を追加し、引数解析をModeenum へ整理した (対処案 c)lib-jj-helpersにquery_remote_bookmarks_at/BookmarkSearch/get_jj_bookmarks_with_remote_fallbackを追加 (既存 API は不変)lib.rsをbookmarks.rs/workspace.rsへ分割 (lib.rsは再エクスポートのファサード、呼び出し側のパスは不変)Context
Why: bot が作った PR を人間がマージする経路 (ADR-072 の夜間ループ) では、PR の head が
claude/nightly-163@originのような リモート専用 bookmark しか持たない。fetch しただけの bookmark はローカルに作られない (jj のgit.auto-local-bookmark既定値) ため、ローカル bookmark だけを見ていた PR 検出が空振りし、pnpm merge-prが「PR が見つかりません」で exit 1 していた。Trigger: #381 のマージで実測 (2026-08-10)。回避策の
jj bookmark trackは非自明で、採用率測定のため夜間 PR をマージするたびに要求される。さらにgh pr mergeは ADR-013 の guard でブロックされるため、ブロックされる経路と動かない経路しかない状態になっていた。Scope decision:
BookmarkSearchでLocal/RemoteOnlyを型で区別した。bookmark を 書き換える 経路 (push-runner の bookmark 前進など) がリモート専用 bookmark を対象にしてはならないためPR_SIZE_CHECK_OVERRIDE=1で override した。diff 2203 行のうち約 1700 行は 800 行 linter に強制されたlib.rsの移動で、実質のレビュー対象は約 500 行。分割しても移動のみの PR が単独で約 1900 行になり閾値を超えるため、分割では解決しない (2026-08-11 ユーザー判断)strip_windows_verbatim_prefixの UNC 対応 1 箇所のみ (レビュー指摘、下記 Review rounds)Validation
cargo test --workspace: 全 crate green (lib-jj-helpers51 pass /cli-merge-pipeline71 pass)cargo test --workspace -- --ignored --test-threads=1: 23 pass (実 jj を spawn する統合テスト)cargo clippy --workspace --all-targets: 警告 0pnpm lint:docs/ markdownlint: 0 errorverdict=APPROVE(DRY 指摘の修正後に再実行、2026-08-11)feat/x@origin [new] untrackedとなりローカル bookmark が作られない、という jj 0.42.0 の実挙動を再現し、従来 API は空 / 新 API はRemoteOnly(["feat/x"])を返すことを固定cargo fmtは全体実行していない (順位 411 の通り無関係な差分を生むため)。新規記述行のみ--checkの指摘に手で合わせたReview rounds
pre-push review (takt): 3 回実行し最終
verdict=APPROVE。指摘して修正したもの:get_jj_bookmarks_with_remote_fallbackがselect_with_remote_fallbackを再実装していた → 委譲に変更 (DRY)workspace.rsは挙動を 1 行も変えていない」と書いたが、同 PR で UNC 分岐を入れており矛盾していた → 記述を線引きの説明に訂正CodeRabbit (Minor 5 件): 3 件修正 / 2 件は根拠つきで見送り (スレッドに返信済み)。
0を引数エラーとして拒否--pr 0/--feedback-only 0が exit 2)。通すとgh pr view 0の失敗が警告に化けてマージ試行まで進むlib.rsの再エクスポート説明が実際より広い (pipeline_lockは非再エクスポート)\\?\UNC\) を一律に剥がすと不正なパスになるgh pr viewは今も検出 1 段目に残っており、「2 段」はローカル→リモートの軸で revset の梯子 (@/@-/@--) とは直交する。両段ともBOOKMARK_SEARCH_REVSETSを渡しているresolve_main_workspace_rootの colocated 経路も canonicalizeFollow-up (別 todo へ登録済み)
resolve_main_workspace_rootの colocated 経路と file 経路で正規化の粒度が違う (💎 Tier 3)CwdRestoreDrop guard が 8 定義 / 6 ファイルに複製され、ADR-025 自身が定めた「2 例目でlib-test-helpersへ統合」トリガーと再評価期限 (2026-07-31) を超過している (🔧 Tier 2)References
docs/todo21.md/docs/todo-summary2.mdから削除済)、WP-18 の運用問題 (docs/harness-improvement-plan.md)