fix(cli-pr-monitor): jj 環境での PR 作成時に --head を自動補完 - #34
Closed
aloekun wants to merge 3 commits into
Closed
Conversation
run_create_pr() で --head 未指定時に get_jj_bookmarks() で jj bookmark を 自動検出し gh pr create に補完する。jj 環境では gh CLI が current branch を 検出できず 'not on any branch' エラーになる問題を解消。 monitor-only モードには同等のフォールバック実装済み (get_pr_info の Strategy B)。 PR 作成モードにも同じパターンを適用。
PR#33 で得られた知見を ADR として記録: - ADR-016: Bash ツールのデフォルト 120s タイムアウト問題。timeout: 600000 + run_in_background が必須 - ADR-017: takt 0.35.4 の Windows 互換性問題。キャレットなし固定 + takt-test-vc で事前検証
- post-pr-create-review-check スキル: exe 名を cli-pr-monitor に修正 (ADR-012 反映漏れ) - pre-push-review スキル: takt 導入済みプロジェクトでのフォールバック位置づけを明記 - templates/push-runner-config.toml: 派生プロジェクト向けテンプレート追加 - deploy-hooks.ts: push-runner-config.toml 未配置の注意表示を追加 - docs/todo.md: PR#33 後の改善タスク + マージ後フィードバック定常化を記録
📝 Walkthrough概要プロジェクトに 2 つの新しいアーキテクチャ決定記録（ADR-016、ADR-017）を追加し、Claude Code Bash での長時間コマンド実行戦略と takt バージョン固定に関するガイダンスを整理。あわせて PR 監視ツールの自動 HEAD 選択機能と デプロイ時の設定検証機能を実装。 変更内容
見積もり レビュー工数🎯 2 (Simple) | ⏱️ ~12 分 関連する可能性のある PR
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
![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 (1)
src/cli-pr-monitor/src/main.rs (1)
711-718: 複数 bookmark 時の先頭固定補完は誤った head 選択につながり得ます。
bookmarks.first()の無条件採用は、複数 bookmark が付いている change で意図しない PR 作成リスクがあります。1件のみ自動補完し、複数時は明示指定を促す分岐が安全です。♻️ リファクタ案
- if let Some(bookmark) = bookmarks.first() { - log_info(&format!( - "jj bookmark '{}' を --head に自動補完", - bookmark - )); - final_args.push("--head".to_string()); - final_args.push(bookmark.clone()); - } + match bookmarks.as_slice() { + [bookmark] => { + log_info(&format!( + "jj bookmark '{}' を --head に自動補完", + bookmark + )); + final_args.push("--head".to_string()); + final_args.push(bookmark.clone()); + } + [] => {} + _ => { + log_info("jj bookmark が複数見つかりました。--head を明示指定してください"); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli-pr-monitor/src/main.rs` around lines 711 - 718, Currently the code unconditionally uses bookmarks.first() to auto-complete --head, which can pick the wrong bookmark when multiple bookmarks exist; change the logic around bookmarks.first() and final_args so that you only auto-complete when bookmarks.len() == 1 (use that single bookmark, push "--head" and bookmark as now), and when bookmarks.len() > 1 do not pick the first one—instead log a message via log_info (or similar) asking the user to explicitly specify the desired head/ bookmark and do not modify final_args; keep existing behavior when bookmarks.is_empty() unchanged. Ensure you reference the bookmarks.first() check, the final_args pushes, and the log_info call when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adr/adr-016-long-running-command-strategy.md`:
- Around line 41-44: The fenced code block containing the lines "timeout:
600000" and "run_in_background: true" is missing a language tag (MD040); update
that fenced block in adr-016-long-running-command-strategy.md to include a
language identifier such as "yaml" (e.g., change ``` to ```yaml) so markdownlint
no longer reports the issue and the snippet is syntax-highlighted correctly.
In `@docs/todo.md`:
- Around line 16-18:
ドキュメントの進捗チェックが未更新になっているので、該当チェックリスト項目を完了済みに変更してください：`cli-pr-monitor: jj 環境での PR
作成時 --head 自動補完`、`ADR-016: Claude Code Bash ツールでの長時間コマンド実行戦略`、`ADR-017: takt
バージョン固定と検証環境の維持` の各行先頭の `[ ]` を `[x]` に置き換え、変更後の todo.md をコミットして PR
に反映してください（該当テキストはファイル内の該当見出し文言で検索して特定できます）。
In `@src/cli-pr-monitor/src/main.rs`:
- Around line 709-719: The current check only looks for exact "--head" in
final_args and misses "--head=<value>" forms; update the presence check in the
block that calls get_jj_bookmarks()/log_info() so it treats any argument that
equals "--head" or starts_with("--head=") as present (e.g., change the any(|a| a
== "--head") predicate to any(|a| a == "--head" || a.starts_with("--head="))).
This ensures you only auto-complete with get_jj_bookmarks() when neither
"--head" nor "--head=<value>" was provided; keep the rest of the logic that
pushes "--head" and the bookmark as-is.
---
Nitpick comments:
In `@src/cli-pr-monitor/src/main.rs`:
- Around line 711-718: Currently the code unconditionally uses bookmarks.first()
to auto-complete --head, which can pick the wrong bookmark when multiple
bookmarks exist; change the logic around bookmarks.first() and final_args so
that you only auto-complete when bookmarks.len() == 1 (use that single bookmark,
push "--head" and bookmark as now), and when bookmarks.len() > 1 do not pick the
first one—instead log a message via log_info (or similar) asking the user to
explicitly specify the desired head/ bookmark and do not modify final_args; keep
existing behavior when bookmarks.is_empty() unchanged. Ensure you reference the
bookmarks.first() check, the final_args pushes, and the log_info call when
making the change.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0b323ba-3337-4920-9ee2-4c9a9fd5fe6c
📒 Files selected for processing (7)
CLAUDE.mddocs/adr/adr-016-long-running-command-strategy.mddocs/adr/adr-017-takt-version-pinning.mddocs/todo.mdscripts/deploy-hooks.tssrc/cli-pr-monitor/src/main.rstemplates/push-runner-config.toml
Comment on lines
+41
to
+44
| ``` | ||
| timeout: 600000 | ||
| run_in_background: true | ||
| ``` |
There was a problem hiding this comment.
コードフェンスに言語指定がありません（MD040）。
markdownlint 警告どおり、フェンスに言語を付けてください。
🛠 修正案
-```
+```yaml
timeout: 600000
run_in_background: true</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/adr/adr-016-long-running-command-strategy.md` around lines 41 - 44, The
fenced code block containing the lines "timeout: 600000" and "run_in_background:
true" is missing a language tag (MD040); update that fenced block in
adr-016-long-running-command-strategy.md to include a language identifier such
as "yaml" (e.g., change ``` to ```yaml) so markdownlint no longer reports the
issue and the snippet is syntax-highlighted correctly.
Comment on lines
+16
to
+18
| - [ ] **cli-pr-monitor: jj 環境での PR 作成時 --head 自動補完**: `run_create_pr()` で `--head` 未指定時に `get_jj_bookmarks()` で jj bookmark を自動検出し補完する。monitor-only モードには同等のフォールバック実装済み。毎回の PR 作成で手動回避が必要な状態のため優先度高 | ||
| - [ ] **ADR-016: Claude Code Bash ツールでの長時間コマンド実行戦略**: デフォルト 120s タイムアウトでプロセスが kill される問題。`timeout: 600000` + `run_in_background: true` を長時間コマンドに必須とする方針を ADR として記録 | ||
| - [ ] **ADR-017: takt バージョン固定と検証環境の維持**: takt 0.35.4 で Windows 環境が壊れた実績。キャレットなし固定 + takt-test-vc を検証環境として位置づける方針を ADR として記録 |
There was a problem hiding this comment.
このPRで完了した項目のチェック状態が未更新です。
--head 自動補完実装、および ADR-016/017 追加が入っているため、進捗管理の正確性のために [x] へ更新した方がよいです。
📝 更新案
-- [ ] **cli-pr-monitor: jj 環境での PR 作成時 --head 自動補完**: `run_create_pr()` で `--head` 未指定時に `get_jj_bookmarks()` で jj bookmark を自動検出し補完する。monitor-only モードには同等のフォールバック実装済み。毎回の PR 作成で手動回避が必要な状態のため優先度高
-- [ ] **ADR-016: Claude Code Bash ツールでの長時間コマンド実行戦略**: デフォルト 120s タイムアウトでプロセスが kill される問題。`timeout: 600000` + `run_in_background: true` を長時間コマンドに必須とする方針を ADR として記録
-- [ ] **ADR-017: takt バージョン固定と検証環境の維持**: takt 0.35.4 で Windows 環境が壊れた実績。キャレットなし固定 + takt-test-vc を検証環境として位置づける方針を ADR として記録
+- [x] **cli-pr-monitor: jj 環境での PR 作成時 --head 自動補完**: `run_create_pr()` で `--head` 未指定時に `get_jj_bookmarks()` で jj bookmark を自動検出し補完する。monitor-only モードには同等のフォールバック実装済み。毎回の PR 作成で手動回避が必要な状態のため優先度高
+- [x] **ADR-016: Claude Code Bash ツールでの長時間コマンド実行戦略**: デフォルト 120s タイムアウトでプロセスが kill される問題。`timeout: 600000` + `run_in_background: true` を長時間コマンドに必須とする方針を ADR として記録
+- [x] **ADR-017: takt バージョン固定と検証環境の維持**: takt 0.35.4 で Windows 環境が壊れた実績。キャレットなし固定 + takt-test-vc を検証環境として位置づける方針を ADR として記録📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - [ ] **cli-pr-monitor: jj 環境での PR 作成時 --head 自動補完**: `run_create_pr()` で `--head` 未指定時に `get_jj_bookmarks()` で jj bookmark を自動検出し補完する。monitor-only モードには同等のフォールバック実装済み。毎回の PR 作成で手動回避が必要な状態のため優先度高 | |
| - [ ] **ADR-016: Claude Code Bash ツールでの長時間コマンド実行戦略**: デフォルト 120s タイムアウトでプロセスが kill される問題。`timeout: 600000` + `run_in_background: true` を長時間コマンドに必須とする方針を ADR として記録 | |
| - [ ] **ADR-017: takt バージョン固定と検証環境の維持**: takt 0.35.4 で Windows 環境が壊れた実績。キャレットなし固定 + takt-test-vc を検証環境として位置づける方針を ADR として記録 | |
| - [x] **cli-pr-monitor: jj 環境での PR 作成時 --head 自動補完**: `run_create_pr()` で `--head` 未指定時に `get_jj_bookmarks()` で jj bookmark を自動検出し補完する。monitor-only モードには同等のフォールバック実装済み。毎回の PR 作成で手動回避が必要な状態のため優先度高 | |
| - [x] **ADR-016: Claude Code Bash ツールでの長時間コマンド実行戦略**: デフォルト 120s タイムアウトでプロセスが kill される問題。`timeout: 600000` + `run_in_background: true` を長時間コマンドに必須とする方針を ADR として記録 | |
| - [x] **ADR-017: takt バージョン固定と検証環境の維持**: takt 0.35.4 で Windows 環境が壊れた実績。キャレットなし固定 + takt-test-vc を検証環境として位置づける方針を ADR として記録 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/todo.md` around lines 16 - 18,
ドキュメントの進捗チェックが未更新になっているので、該当チェックリスト項目を完了済みに変更してください：`cli-pr-monitor: jj 環境での PR
作成時 --head 自動補完`、`ADR-016: Claude Code Bash ツールでの長時間コマンド実行戦略`、`ADR-017: takt
バージョン固定と検証環境の維持` の各行先頭の `[ ]` を `[x]` に置き換え、変更後の todo.md をコミットして PR
に反映してください（該当テキストはファイル内の該当見出し文言で検索して特定できます）。
Comment on lines
+709
to
+719
| if !final_args.iter().any(|a| a == "--head") { | ||
| let bookmarks = get_jj_bookmarks(); | ||
| if let Some(bookmark) = bookmarks.first() { | ||
| log_info(&format!( | ||
| "jj bookmark '{}' を --head に自動補完", | ||
| bookmark | ||
| )); | ||
| final_args.push("--head".to_string()); | ||
| final_args.push(bookmark.clone()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub CLI の gh pr create で、--head=形式は正式サポートされていますか？また--head の重複指定時の優先ルール（先勝ち/後勝ち/エラー）を確認してください。
💡 Result:
GitHub CLIのgh pr createコマンドで--head=形式は正式にサポートされています。公式マニュアルで-H, --head として記載されており、デフォルトは現在のブランチです。形式はowner:branchもサポートされ、フォークからのPR作成に使用されます（例: --head monalisa:feature）。 --headの重複指定時の優先ルールについては、ソースコードおよびドキュメントで明示的に記載されていません。GitHub CLIはCobraライブラリを使用しており、一般的にstring型のフラグは最後に指定された値（後勝ち）が優先されます。重複指定でエラーが発生するという情報も見つかりませんでした。
Citations:
- 1: https://cli.github.com/manual/gh_pr_create
- 2: https://github.com/cli/cli/blob/trunk/pkg/cmd/pr/create/create.go
- 3: https://fossies.org/linux/gh-cli/pkg/cmd/pr/create/create.go
- 4: https://stackoverflow.com/questions/67100626/how-to-create-a-pull-request-via-github-cli-with-multiple-assignees
--head=<value> 形式の指定を見落として自動補完が走る問題があります。
コードが "--head" の完全一致のみをチェックしているため、ユーザーが --head=feature-x のような形式でオプションを指定した場合、自動補完ロジックがこれを検出できず、ジジ倉庫のブックマークを追加してしまいます。GitHub CLI では最後に指定された値が優先されるため、ユーザーの意図が無視されて、意図しないブランチが PR のヘッドになります。
修正例
- if !final_args.iter().any(|a| a == "--head") {
+ let has_head = final_args
+ .iter()
+ .any(|a| a == "--head" || a.starts_with("--head="));
+ if !has_head {
let bookmarks = get_jj_bookmarks();
if let Some(bookmark) = bookmarks.first() {
log_info(&format!(
"jj bookmark '{}' を --head に自動補完",
bookmark
));
final_args.push("--head".to_string());
final_args.push(bookmark.clone());
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if !final_args.iter().any(|a| a == "--head") { | |
| let bookmarks = get_jj_bookmarks(); | |
| if let Some(bookmark) = bookmarks.first() { | |
| log_info(&format!( | |
| "jj bookmark '{}' を --head に自動補完", | |
| bookmark | |
| )); | |
| final_args.push("--head".to_string()); | |
| final_args.push(bookmark.clone()); | |
| } | |
| } | |
| let has_head = final_args | |
| .iter() | |
| .any(|a| a == "--head" || a.starts_with("--head=")); | |
| if !has_head { | |
| let bookmarks = get_jj_bookmarks(); | |
| if let Some(bookmark) = bookmarks.first() { | |
| log_info(&format!( | |
| "jj bookmark '{}' を --head に自動補完", | |
| bookmark | |
| )); | |
| final_args.push("--head".to_string()); | |
| final_args.push(bookmark.clone()); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli-pr-monitor/src/main.rs` around lines 709 - 719, The current check
only looks for exact "--head" in final_args and misses "--head=<value>" forms;
update the presence check in the block that calls get_jj_bookmarks()/log_info()
so it treats any argument that equals "--head" or starts_with("--head=") as
present (e.g., change the any(|a| a == "--head") predicate to any(|a| a ==
"--head" || a.starts_with("--head="))). This ensures you only auto-complete with
get_jj_bookmarks() when neither "--head" nor "--head=<value>" was provided; keep
the rest of the logic that pushes "--head" and the bookmark as-is.
Owner
Author
|
PR を個別ブランチに分割して再作成します。jj bookmark スタックにより3コミットが1 PR にまとまってしまったため。 |
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
Summary by CodeRabbit
リリースノート
Documentation
New Features
--headオプションを補完