feat(jj-workspace): 非 colocated workspace の gh 全損を GIT_DIR 自動注入で解消 + GH_REPO guard (順位251) - #239
Merged
Merged
Conversation
…位251) PR #238 で実観測した 3 欠陥 (repo 検出失敗 / checker JSON parse 停止 / post-merge feedback silent 消失) と、GH_REPO による場当たり対処の部分故障を 構造的に解消する (ADR-045 恒久対策候補 1 の実装 + 防御 preset)。 (a) GIT_DIR 自動注入 (lib-jj-helpers::inject_git_dir_for_gh): - .git 不在 + GIT_DIR 未設定のとき .jj/repo (secondary workspace では main store への相対パスファイル) → store/git_target を辿って main の .git を導出し、プロセス env に設定 (子プロセス gh 全体へ伝播) - cli-pr-monitor / cli-merge-pipeline / check-ci-coderabbit の main() で 注入。既存 env 尊重・導出失敗は warning + 続行 (fail-soft) - tempdir 疑似 layout の unit test 6 件 + 実 jj (init --colocate + workspace add) の #[ignore] 統合テスト (d) gh-repo-env-guard preset (hooks-pre-tool-validate、恒久): - Bash / PowerShell の GH_REPO 環境変数代入を block し GIT_DIR / 自動注入 / repo 指定フラグへ誘導 (PowerShell 構文は matcher 拡張に備え先行) - GH_REPO は引数なし gh repo view に効かず silent 部分故障を招くため (b) checker 出力の stdout/stderr 分離 (run_cmd_capture 新設): - invoke_checker は stdout のみ JSON parse、stderr は log 転送。checker の fail-soft stderr 警告が JSON に連結され trailing characters で監視停止 した回帰の防止 (cmd 実プロセスの regression test 付き) (c) owner_repo 検出失敗時の .failed marker (AiStepContext::SkipWithMarker): - 従来は marker なしで skip し L2 recovery (ADR-030) が発火しなかった。 skip でも marker を書き recovery 可能に docs: ADR-045 改訂 (候補 1 実装済み化、手動 GIT_DIR を fallback に格下げ、 コマンド対応表更新)、todo13/todo-summary 順位 251 現在地更新 (dogfood 記録 と順位 225 dogfood 開始日記入を含む) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthrough非colocated jj workspaceでのgh呼び出し失敗に対応するため、lib-jj-helpersにGIT_DIR解決・注入APIを新設し、cli-pr-monitor / cli-merge-pipeline / check-ci-coderabbitに組み込んだ。checker出力のstdout/stderr分離、marker付きAIステップスキップ、GH_REPOをブロックするhooksプリセット、関連ドキュメント更新も含む。 ChangesGIT_DIR自動注入とGH_REPOガード
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
🧹 Nitpick comments (2)
src/hooks-pre-tool-validate/src/presets/gh.rs (1)
57-70: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
printf -v/read経由の代入はすり抜ける可能性があります。
printf -v GH_REPO "..."やread GH_REPO <<< ...のように=を伴わない代入方法は現状のパターンでは検出されません。ただし、本ファイルの他プリセット (gh-pr-create-guard等) も同様に単純な正規表現ベースの検出方針を取っており、実運用上のリスクは低いと考えます。Based on learnings, this repo's hook validator presets intentionally use single-pattern regex detection rather than tracking broader command semantics, so this is a minor, acceptable gap rather than a required fix.
🤖 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/hooks-pre-tool-validate/src/presets/gh.rs` around lines 57 - 70, The GH_REPO guard in preset_gh_repo_env_guard intentionally uses simple regex-based matching, so this is not a functional bug to expand into command-semantic parsing; keep the existing BlockedPattern rules consistent with the other gh presets. If you want to address the comment, add a brief note near preset_gh_repo_env_guard (or the GH_REPO_ENV_MSG usage) clarifying that forms like printf -v and read are intentionally out of scope for this validator.Source: Learnings
src/lib-jj-helpers/Cargo.toml (1)
12-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
tempfileはワークスペース依存関係経由での宣言を検討してください。As per coding guidelines,
**/Cargo.tomlは "Integrate Rust packages through a Cargo workspace" を求めています。本変更はtempfile = "3"をこの crate の[dev-dependencies]に直接バージョン指定していますが、他の crate (下流ファイルの graph context 上でもtempfileへの参照が示唆されている) と重複してバージョンが乖離するリスクがあります。ルートの[workspace.dependencies]にtempfileを追加し、tempfile = { workspace = true }で参照する形に統一することを推奨します。♻️ ワークスペース依存関係化の例 (ルート Cargo.toml が既にワークスペースの場合)
[dev-dependencies] -tempfile = "3" +tempfile = { workspace = true }以下のスクリプトでルート
Cargo.tomlの[workspace.dependencies]にtempfileが既に存在するか確認できます:#!/bin/bash echo "--- root workspace Cargo.toml の [workspace.dependencies] ---" fd -HI '^Cargo\.toml$' -d 1 --exec cat {} \; echo "--- 他 crate での tempfile 依存宣言方法 ---" rg -n 'tempfile' --type=toml🤖 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/Cargo.toml` around lines 12 - 14, The current dev-dependency declaration for tempfile is hardcoded in this crate, so update it to use the workspace-managed dependency instead. Check the root Cargo.toml for [workspace.dependencies], add tempfile there if missing, and change this crate’s [dev-dependencies] entry in the Cargo.toml for src/lib-jj-helpers to reference tempfile with workspace = true. Keep the dependency name consistent with other crates that use tempfile so version ownership stays centralized.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/hooks-pre-tool-validate/src/presets/gh.rs`:
- Around line 57-70: The GH_REPO guard in preset_gh_repo_env_guard intentionally
uses simple regex-based matching, so this is not a functional bug to expand into
command-semantic parsing; keep the existing BlockedPattern rules consistent with
the other gh presets. If you want to address the comment, add a brief note near
preset_gh_repo_env_guard (or the GH_REPO_ENV_MSG usage) clarifying that forms
like printf -v and read are intentionally out of scope for this validator.
In `@src/lib-jj-helpers/Cargo.toml`:
- Around line 12-14: The current dev-dependency declaration for tempfile is
hardcoded in this crate, so update it to use the workspace-managed dependency
instead. Check the root Cargo.toml for [workspace.dependencies], add tempfile
there if missing, and change this crate’s [dev-dependencies] entry in the
Cargo.toml for src/lib-jj-helpers to reference tempfile with workspace = true.
Keep the dependency name consistent with other crates that use tempfile so
version ownership stays centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d0dc07b-be85-4f23-b6ff-cf571bf43424
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.claude/hooks-config.tomldocs/adr/adr-045-jj-workspace-parallel-sessions.mddocs/auto-push-gate-dogfood.mddocs/todo-summary.mddocs/todo13.mdsrc/check-ci-coderabbit/Cargo.tomlsrc/check-ci-coderabbit/src/main.rssrc/cli-merge-pipeline/src/main.rssrc/cli-merge-pipeline/src/pipeline.rssrc/cli-pr-monitor/src/main.rssrc/cli-pr-monitor/src/runner.rssrc/cli-pr-monitor/src/stages/poll/iteration.rssrc/hooks-pre-tool-validate/src/presets/gh.rssrc/hooks-pre-tool-validate/src/presets/mod.rssrc/lib-jj-helpers/Cargo.tomlsrc/lib-jj-helpers/src/lib.rs
aloekun
added a commit
that referenced
this pull request
Jul 3, 2026
…252/253 登録) (#240) - 順位 251 (非 colocated jj workspace 対応) を完了削除: PR #239 で GIT_DIR 自動注入 + gh-repo-env-guard preset + stdout 分離 + skip 時 marker として land、dogfood (監視完走 + post-merge feedback 復旧) を 実運用確認済みのため entry + summary 行を削除 - 順位 252 登録: 部分効果 env var anti-pattern の文書化 (新 ADR、 placeholder policy 適用、feedback T3-1 採用) - 順位 253 登録: ADR-030 に feedback silent skip 実装記録を追記 (feedback T3-2 採用) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
概要
PR #238 の監視・マージで実観測した「非 colocated jj workspace での gh 全損」(順位 251) を構造的に解消する。ADR-045 §恒久対策の候補 1 (
GIT_DIR自動注入) の実装 + 場当たり env 対処への防御 preset + 監視 parse / recovery の独立防御 2 層。変更内容
(a)
GIT_DIR自動注入 —lib_jj_helpers::inject_git_dir_for_gh新設.git不在 +GIT_DIR未設定のとき、.jj/repo(secondary workspace では main store への相対パスを格納したファイル) →store/git_targetの順に辿って main の.gitを導出し、プロセス env に設定 (子プロセス gh 全体へ伝播)cli-pr-monitor/cli-merge-pipeline/check-ci-coderabbitの main() 冒頭で注入。既存 env 尊重・colocated 環境では no-op・導出失敗は warning + 続行 (fail-soft)init --colocate+workspace add) の#[ignore]統合テスト (jj 0.42.0 の実レイアウトで検証)(d)
gh-repo-env-guardpreset (hooks-pre-tool-validate、恒久)gh repo view(exe 群の repo 検出) には無効 → 「マージ成功・feedback 消失」の silent 部分故障を招く (PR feat(cli-pr-monitor): auto-push gate-bypass 是正 PR-1 — A1 fix facet --ignored ゲート + B1 auto-push 前 quality gate (順位225) #238 実観測)(b) checker 出力の stdout/stderr 分離 —
run_cmd_capture新設invoke_checkerは stdout のみを JSON parse し、stderr は log 転送。checker の fail-soft stderr 警告が JSON に連結され「trailing characters」で監視停止した回帰の防止run_cmd_directは capture 版への委譲に refactor (既存挙動保持のテスト付き)、cmd 実プロセスの regression test 3 件(c) owner_repo 検出失敗時の
.failedmarker —AiStepContext::SkipWithMarkerdocs
GIT_DIR前置を fallback に格下げ、コマンド対応表更新、GH_REPO 不可の明記検証
cargo clippy --workspace -- -D warningsPASS /cargo test全 29 suite PASS /cargo test -- --ignored --test-threads=115 件 PASS /pnpm lint:docsOKDogfood 証跡 (本 PR 自体が実地テスト)
check-ci-coderabbit.exeを env 前置なしで実行 →[env] GIT_DIR 自動注入 (非 colocated jj workspace): C:\Users\owner\work\claude-code-hook-test\.gitが stderr に出力され、従来の「初期化エラー: リポジトリ取得失敗」が消滅。本 PR の push / PR 作成 / 監視もすべて素のコマンドで実行GH_REPO=... gh api ...を意図的に実行 → 誘導メッセージ付きで block を確認GH_REPO=の字面を含めた際に guard が発火 (text-scanning hook の既知 FP クラス)。字面を避ける表現に修正して回避 — 発生頻度は低く運用で許容と判断🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
gh関連コマンドで必要な環境設定を自動補完しやすくなりました。GH_REPOの直接利用を避ける案内が追加されました。バグ修正
ドキュメント