feat: repo-maintenance に推奨ファイル同期ステップを追加 - #597
Conversation
Step 3.22 として Config Template Sync を追加。 config リポジトリの推奨ワークフローやフックファイルを 他のリポジトリに自動同期する機能。 マネージドファイル（claude.yml, hooks等）は常に最新版で上書き、 テンプレートファイル（ci.yml, security.yml等）は差分確認後に更新。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new Setup sub-step "Config Template Sync" that compares selected config-managed/template files against Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Runner / CI
participant RepoCmd as repo-maintenance command
participant RepoFS as Target Repo FS
participant ConfigSrc as keito4/config
Runner->>RepoCmd: invoke "Config Template Sync"
RepoCmd->>RepoFS: read repo metadata (.devcontainer/devcontainer.json)
alt repo is keito4/config
RepoCmd->>Runner: skip sync (self-repo)
else not under config-base
RepoCmd->>Runner: skip sync (not config-base)
else proceed
RepoCmd->>ConfigSrc: fetch listed template/managed files
ConfigSrc-->>RepoCmd: return file contents
RepoCmd->>RepoFS: for each file
loop per-file policy
RepoCmd->>RepoFS: check existence & diff
alt managed file (.github/workflows/claude*.yml, .claude/hooks/*.py)
RepoCmd->>RepoFS: copy if missing or different
else template file (.github/workflows/security.yml, ci.yml)
RepoCmd->>Runner: show diff
alt MODE=full
RepoCmd->>RepoFS: overwrite with source
else
RepoCmd->>Runner: report diff only
end
else issue/pr templates (.github/ISSUE_TEMPLATE/*, pull_request_template.md)
RepoCmd->>RepoFS: copy missing entries only
end
end
RepoCmd->>Runner: update Setup progress line "Config Template Sync: …"
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
🔍 AI Code Review (Local Hook)Models: Codex (default) / Gemini (default) 🤖 Codex Review指摘事項
全体判定
|
🔍 AI Code Review (Local Hook)Models: Codex (default) / Gemini (default) 🤖 Codex Review以下は変更差分に対するレビューです。
判定: patch is incorrect
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be4a893d05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| echo "上書きしますか？ (y/n)" | ||
| # Claude による対話的確認 | ||
| cp "$SRC" "$DST" | ||
| UPDATED+=("$file (更新)") |
There was a problem hiding this comment.
This branch asks the user whether to overwrite (上書きしますか？) but then immediately copies the source file without reading or checking any response, so full-mode runs will always overwrite customized ci.yml/security.yml once a diff is found. That makes the confirmation prompt ineffective and can silently discard repository-specific workflow changes.
Useful? React with 👍 / 👎.
| elif ! diff -q "$SRC" "$DST" >/dev/null 2>&1; then | ||
| # 差分あり → 上書き | ||
| diff --color=auto -u "$DST" "$SRC" | head -30 | ||
| cp "$SRC" "$DST" | ||
| UPDATED+=("$file (更新)") |
There was a problem hiding this comment.
The mode contract at the top says quick and check-only should not update files, but this managed-file sync path performs cp as soon as a diff is detected with no MODE check. As written, non-full runs can still modify the working tree, which breaks the command’s read-only/check semantics and can create unexpected changes during audit-only usage.
Useful? React with 👍 / 👎.
|
Claude Code レビュー PR の目的・背景は明確で有益な機能追加です。ただし、スクリプトロジックにいくつか問題点があります。 バグ・重大な問題 1. テンプレートファイルの確認が機能していない 2. MODE 変数が未定義 3. TMPDIR 変数のシャドウイング 4. 一時ディレクトリのリーク 注意点 5. 空ディレクトリでの glob: for src_file in "$SRC_DIR"/* は空ディレクトリで * がリテラル展開されます。 6. diff --color=auto: 非インタラクティブ環境では --color=never の方が安全です。 良い点 まとめ 優先度: 必須 → MODE 定義の明記 or 確認ロジック削除 / trap によるクリーンアップ / TMPDIR リネーム テンプレートファイルの確認ロジックが実装と矛盾している点を中心に、マージ前に修正をお願いします。 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.claude/commands/repo-maintenance.md (2)
1840-1840: Consider showing full diff or configurable limit.The diff output is truncated to 30 lines with
head -30. For managed files with extensive changes, this might hide important modifications from the user. Consider either showing the full diff, using a larger limit, or making it configurable.💡 Alternative approaches
Option 1: Show full diff
- diff --color=auto -u "$DST" "$SRC" | head -30 + diff --color=auto -u "$DST" "$SRC"Option 2: Use a pager for large diffs
- diff --color=auto -u "$DST" "$SRC" | head -30 + DIFF_OUTPUT=$(diff --color=auto -u "$DST" "$SRC") + LINE_COUNT=$(echo "$DIFF_OUTPUT" | wc -l) + if [ "$LINE_COUNT" -gt 50 ]; then + echo "$DIFF_OUTPUT" | less -R + else + echo "$DIFF_OUTPUT" + fiOption 3: Configurable limit
+ DIFF_LINES=${DIFF_PREVIEW_LINES:-30} - diff --color=auto -u "$DST" "$SRC" | head -30 + diff --color=auto -u "$DST" "$SRC" | head -n "$DIFF_LINES"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/repo-maintenance.md at line 1840, The current command truncates diffs with a hard-coded head -30 which can hide important changes; update the invocation that uses diff --color=auto -u "$DST" "$SRC" | head -30 to either remove the head pipe to show the full diff, or replace the fixed limit with a configurable variable (e.g., DIFF_LIMIT) and only pipe to head when that variable is set, or instead pipe to a pager (LESS/most) when running interactively; locate the invocation referencing "$DST" and "$SRC" and change the pipeline accordingly so the limit is not hard-coded.
1754-1932: Consider adding backup mechanism for overwritten files.The managed files sync overwrites local files without creating backups. If a repository has legitimate customizations to managed files (e.g., for testing or temporary fixes), those changes will be lost without warning.
Consider adding an optional backup mechanism:
🔄 Proposed enhancement
BACKUP_DIR=".git/config-sync-backup/$(date +%Y%m%d-%H%M%S)" for file in "${MANAGED_FILES[@]}"; do SRC="$CONFIG_REPO/$file" DST="./$file" if [ ! -f "$SRC" ]; then continue fi mkdir -p "$(dirname "$DST")" if [ ! -f "$DST" ]; then cp "$SRC" "$DST" UPDATED+=("$file (新規追加)") elif ! diff -q "$SRC" "$DST" >/dev/null 2>&1; then # Create backup before overwriting mkdir -p "$BACKUP_DIR/$(dirname "$file")" cp "$DST" "$BACKUP_DIR/$file" diff --color=auto -u "$DST" "$SRC" | head -30 cp "$SRC" "$DST" UPDATED+=("$file (更新、バックアップ: $BACKUP_DIR/$file)") else SKIPPED+=("$file (最新)") fi doneThis allows users to review and restore customizations if needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/repo-maintenance.md around lines 1754 - 1932, Add an optional backup step before overwriting managed files: define a timestamped BACKUP_DIR (e.g., .git/config-sync-backup/$(date ...)), and in the managed-files loop that iterates over MANAGED_FILES, when you detect a diff (! diff -q "$SRC" "$DST"), create the target backup directory with mkdir -p "$BACKUP_DIR/$(dirname "$file")", copy the existing DST to the backup location (cp "$DST" "$BACKUP_DIR/$file"), then show the diff and proceed to overwrite via cp "$SRC" "$DST"; update the UPDATED entry to include backup path, and ensure the same safe mkdir -p logic is used for new files so backups never fail; make the backup behavior optional via a flag or env var (e.g., BACKUP=true) so it can be toggled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/repo-maintenance.md:
- Around line 1802-1806: Add robust error handling around the GitHub API
fallback block that sets CONFIG_REPO and TMPDIR: ensure the `gh` CLI is present
and the `gh api repos/keito4/config/tarball/main | tar xz -C "$TMPDIR"
--strip-components=1` pipeline succeeds by checking exit statuses, and if either
`gh` or `tar` fails, remove the created TMPDIR, print a clear error to stderr,
and exit non-zero so the script does not continue with an invalid CONFIG_REPO;
reference the CONFIG_REPO and TMPDIR variables and the `gh api ... | tar xz`
pipeline when adding these checks and cleanup.
- Around line 1803-1806: The temporary directory created by TMPDIR=$(mktemp -d)
is not removed; after creating TMPDIR and setting CONFIG_REPO, add a cleanup
trap (e.g., trap 'rm -rf "$TMPDIR"' EXIT or trap with INT TERM for robustness)
so the temp directory is removed when the script exits or is interrupted; place
this trap immediately after TMPDIR is created to ensure CONFIG_REPO and
subsequent gh/tar operations still use the directory but it is always cleaned
up.
- Around line 1877-1882: The prompt "上書きしますか？ (y/n)" is printed but no
confirmation is read; update the block that runs when MODE="full" to actually
read user input (e.g., using read -r answer) and only run cp "$SRC" "$DST" and
append to UPDATED if the answer is "y" or "Y"; for other answers skip copying
(and consider printing a message). Ensure this uses the existing variables MODE,
SRC, DST, and UPDATED and handle non-interactive environments by treating
empty/no input as "no".
- Around line 1763-1765: Add early-exit checks at the top of the first bash sync
block to skip running on the config repo or on repos not managed by config:
detect the current repository identity (e.g. from git remote origin URL or
GITHUB_REPOSITORY/GIT_REPO env) and if it equals "keito4/config" exit 0; then
check for the marker string "config-base" inside .devcontainer/devcontainer.json
(e.g. test -f .devcontainer/devcontainer.json && grep -q '"config-base"'
.devcontainer/devcontainer.json) and if not found exit 0; place these checks
before any main sync logic so the script returns early for those cases.
---
Nitpick comments:
In @.claude/commands/repo-maintenance.md:
- Line 1840: The current command truncates diffs with a hard-coded head -30
which can hide important changes; update the invocation that uses diff
--color=auto -u "$DST" "$SRC" | head -30 to either remove the head pipe to show
the full diff, or replace the fixed limit with a configurable variable (e.g.,
DIFF_LIMIT) and only pipe to head when that variable is set, or instead pipe to
a pager (LESS/most) when running interactively; locate the invocation
referencing "$DST" and "$SRC" and change the pipeline accordingly so the limit
is not hard-coded.
- Around line 1754-1932: Add an optional backup step before overwriting managed
files: define a timestamped BACKUP_DIR (e.g., .git/config-sync-backup/$(date
...)), and in the managed-files loop that iterates over MANAGED_FILES, when you
detect a diff (! diff -q "$SRC" "$DST"), create the target backup directory with
mkdir -p "$BACKUP_DIR/$(dirname "$file")", copy the existing DST to the backup
location (cp "$DST" "$BACKUP_DIR/$file"), then show the diff and proceed to
overwrite via cp "$SRC" "$DST"; update the UPDATED entry to include backup path,
and ensure the same safe mkdir -p logic is used for new files so backups never
fail; make the backup behavior optional via a flag or env var (e.g.,
BACKUP=true) so it can be toggled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2efcdac0-3188-4211-af06-18c26f25d1ed
📒 Files selected for processing (1)
.claude/commands/repo-maintenance.md
| if [ -z "$CONFIG_REPO" ]; then | ||
| TMPDIR=$(mktemp -d) | ||
| CONFIG_REPO="$TMPDIR" | ||
| gh api repos/keito4/config/tarball/main | tar xz -C "$TMPDIR" --strip-components=1 | ||
| fi |
There was a problem hiding this comment.
Add error handling for GitHub API fallback.
The GitHub API tarball fetch and extraction has no error handling. If gh CLI is unavailable, the user lacks access, or the network request fails, the script will continue with an invalid CONFIG_REPO path.
🛡️ Proposed fix to add error handling
if [ -z "$CONFIG_REPO" ]; then
TMPDIR=$(mktemp -d)
+ trap 'rm -rf "$TMPDIR"' EXIT
CONFIG_REPO="$TMPDIR"
- gh api repos/keito4/config/tarball/main | tar xz -C "$TMPDIR" --strip-components=1
+ if ! gh api repos/keito4/config/tarball/main | tar xz -C "$TMPDIR" --strip-components=1 2>/dev/null; then
+ echo "❌ config リポジトリの取得に失敗しました"
+ echo " gh CLI が利用可能か、keito4/config へのアクセス権があるか確認してください"
+ exit 1
+ fi
fi📝 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.
| if [ -z "$CONFIG_REPO" ]; then | |
| TMPDIR=$(mktemp -d) | |
| CONFIG_REPO="$TMPDIR" | |
| gh api repos/keito4/config/tarball/main | tar xz -C "$TMPDIR" --strip-components=1 | |
| fi | |
| if [ -z "$CONFIG_REPO" ]; then | |
| TMPDIR=$(mktemp -d) | |
| trap 'rm -rf "$TMPDIR"' EXIT | |
| CONFIG_REPO="$TMPDIR" | |
| if ! gh api repos/keito4/config/tarball/main | tar xz -C "$TMPDIR" --strip-components=1 2>/dev/null; then | |
| echo "❌ config リポジトリの取得に失敗しました" | |
| echo " gh CLI が利用可能か、keito4/config へのアクセス権があるか確認してください" | |
| exit 1 | |
| fi | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/repo-maintenance.md around lines 1802 - 1806, Add robust
error handling around the GitHub API fallback block that sets CONFIG_REPO and
TMPDIR: ensure the `gh` CLI is present and the `gh api
repos/keito4/config/tarball/main | tar xz -C "$TMPDIR" --strip-components=1`
pipeline succeeds by checking exit statuses, and if either `gh` or `tar` fails,
remove the created TMPDIR, print a clear error to stderr, and exit non-zero so
the script does not continue with an invalid CONFIG_REPO; reference the
CONFIG_REPO and TMPDIR variables and the `gh api ... | tar xz` pipeline when
adding these checks and cleanup.
| if [ "$MODE" = "full" ]; then | ||
| echo "上書きしますか？ (y/n)" | ||
| # Claude による対話的確認 | ||
| cp "$SRC" "$DST" | ||
| UPDATED+=("$file (更新)") | ||
| fi |
There was a problem hiding this comment.
Implement missing interactive confirmation for template files.
The code comment indicates "Claude による対話的確認" (Claude interactive confirmation), but the actual confirmation logic is not implemented. The code prints "上書きしますか？ (y/n)" but then immediately copies the file without waiting for user response. This violates the documented template file policy that requires "差分表示 → 確認後に上書き".
🔧 Proposed fix to implement actual confirmation
if [ "$MODE" = "full" ]; then
echo "上書きしますか？ (y/n)"
- # Claude による対話的確認
- cp "$SRC" "$DST"
- UPDATED+=("$file (更新)")
+ read -r response
+ if [[ "$response" =~ ^[Yy] ]]; then
+ cp "$SRC" "$DST"
+ UPDATED+=("$file (更新)")
+ else
+ echo "スキップしました: $file"
+ SKIPPED+=("$file (ユーザーがスキップ)")
+ fi
fiNote: If this is intended to be handled by Claude Code's interactive capabilities rather than shell read, the documentation should clarify this implementation detail.
📝 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.
| if [ "$MODE" = "full" ]; then | |
| echo "上書きしますか？ (y/n)" | |
| # Claude による対話的確認 | |
| cp "$SRC" "$DST" | |
| UPDATED+=("$file (更新)") | |
| fi | |
| if [ "$MODE" = "full" ]; then | |
| echo "上書きしますか？ (y/n)" | |
| read -r response | |
| if [[ "$response" =~ ^[Yy] ]]; then | |
| cp "$SRC" "$DST" | |
| UPDATED+=("$file (更新)") | |
| else | |
| echo "スキップしました: $file" | |
| SKIPPED+=("$file (ユーザーがスキップ)") | |
| fi | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/repo-maintenance.md around lines 1877 - 1882, The prompt
"上書きしますか？ (y/n)" is printed but no confirmation is read; update the block that
runs when MODE="full" to actually read user input (e.g., using read -r answer)
and only run cp "$SRC" "$DST" and append to UPDATED if the answer is "y" or "Y";
for other answers skip copying (and consider printing a message). Ensure this
uses the existing variables MODE, SRC, DST, and UPDATED and handle
non-interactive environments by treating empty/no input as "no".
- スキップ条件の具体的な判定ロジックを追加 - TMPDIR → TEMP_CONFIG_DIR に変更（システム予約変数のシャドウイング回避） - trap による一時ディレクトリのクリーンアップを追加 - テンプレートファイルの確認ロジックを明確化（MODE=full は Claude が確認、それ以外は報告のみ） - 空ディレクトリでの glob 対策を追加 - diff の --color=auto を削除（非インタラクティブ環境対応） Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.98.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
repo-maintenanceコマンドに Step 3.22「Config Template Sync」を追加背景
/setup-new-repoで初期構築したリポジトリのワークフローやフックは、config リポジトリの更新に追従しない。例えばclaude.ymlのcancel-in-progress修正（#595）のような変更が全リポジトリに反映されない問題があった。同期ポリシー
claude.yml,claude-code-review.yml, Claude hooks (3件)ci.yml,security.ymlTest plan
/repo-maintenance実行時に差分が検出・同期されること🤖 Generated with Claude Code
Summary by CodeRabbit