feat: repo-maintenanceにcheck-file-length自動追加を組み込む - #537
Conversation
huskyが設定済みのリポジトリで/repo-maintenanceを実行した際、 pre-commitフックにcheck-file-lengthが含まれていない場合に自動追加する ステップ(3.2.1)を追加。 - DevContainer環境では/usr/local/script/check-file-length.shをコピー - ローカル環境ではGitHubからcurlで取得 - .filelengthignoreがなければデフォルトテンプレートを生成 - サマリーレポートとRelated Commandsテーブルにも項目追加 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis change adds a new sub-section to the repo-maintenance documentation that establishes automated setup logic for a check-file-length pre-commit hook. The update includes instructions for checking existing hooks, fetching necessary scripts, setting permissions, appending hook entries, and creating/populating a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
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)
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 |
![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: d3335fda08
ℹ️ 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".
| # すでに lint-staged を呼んでいる行の後に追加 | ||
| echo "" >> "$PRECOMMIT" | ||
| echo "# check-file-length" >> "$PRECOMMIT" | ||
| echo "bash script/check-file-length.sh" >> "$PRECOMMIT" |
There was a problem hiding this comment.
This appends the new hook lines to the end of .husky/pre-commit, but in repositories where the hook already ends with exit 0 (or another unconditional exit), the appended bash script/check-file-length.sh never runs even though maintenance reports it as added. Please insert the command before terminal exit statements (or rewrite the hook block) so the check is guaranteed to execute.
Useful? React with 👍 / 👎.
| has_file_length=false | ||
|
|
||
| [ -f "$PRECOMMIT" ] && has_precommit=true | ||
| grep -q "check-file-length" "$PRECOMMIT" 2>/dev/null && has_file_length=true |
There was a problem hiding this comment.
The check marks check-file-length as configured based only on a string match in pre-commit, so a stale or broken hook entry (for example, a path that no longer exists) is treated as ✅ and skipped. In that case maintenance does not repair the setup and commits can still fail or bypass the intended check; the detection should require both hook presence and a valid script target before skipping.
Useful? React with 👍 / 👎.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/commands/repo-maintenance.md (1)
1-5:⚠️ Potential issue | 🟠 MajorMajor: Missing linked issue in PR.
The PR description includes a comprehensive test plan but does not reference a linked issue. Based on learnings, this repository enforces the requirement: "Linked Issue required" as part of PR quality gates. Please link the related issue that tracks this feature request or create one if it doesn't exist.
Based on learnings: PR quality gates require Diff ≤ 400 lines, ≤ 25 files changed, and Linked Issue required.
🤖 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 1 - 5, The PR is missing a required linked issue; update the PR description to include a "Linked Issue" line with the issue URL or number (e.g. "Linked Issue: `#123`" or "Linked Issue: https://...") or create the issue first and then add its link; if creating a new issue, open one with a clear title and brief description summarizing the change and then paste that issue link into the PR body (ensure the PR body contains exactly "Linked Issue: <issue-url-or-#>" so the repo's quality gate recognizes it).
🧹 Nitpick comments (1)
.claude/commands/repo-maintenance.md (1)
366-377: Optional: Consider aligning template handling with existing setup script.Lines 366-377 handle
.filelengthignoretemplate creation, but the approach differs from the existingscript/setup-file-length-check.sh:
- Template path: Uses
/usr/local/share/config-templates/.filelengthignore.templateinstead of$TEMPLATE_DIR/.filelengthignore.template- Inline defaults: Provides a minimal 3-pattern template (lines 371-375), while the existing setup script has a more comprehensive default with 10+ patterns (see context snippet 3 lines 45-58)
These differences might confuse users about which template is canonical. Consider either:
- Using the same template path resolution logic as the existing script
- Expanding the inline template to match the existing defaults
Based on relevant code snippet from script/setup-file-length-check.sh:38-63 showing the existing template handling approach.
🤖 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 366 - 377, The .filelengthignore creation uses a hardcoded IGNORE_TEMPLATE path and a minimal inline default; update this to follow the existing script/setup-file-length-check.sh logic by resolving the template via the same TEMPLATE_DIR (or same resolution function) instead of /usr/local/share/config-templates, and either replace the inline heredoc for .filelengthignore with the comprehensive default patterns from script/setup-file-length-check.sh or copy the canonical template file from TEMPLATE_DIR/.filelengthignore.template when present; adjust references to IGNORE_TEMPLATE and the .filelengthignore heredoc accordingly so the two places stay consistent.
🤖 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 358-360: The pre-commit hook lines being appended (using PRECOMMIT
and "bash script/check-file-length.sh") lack the logging echo and the "|| exit
1" failure propagation used by script/setup-file-length-check.sh; update the
invocation to match the existing pattern by appending an echo logging line like
the one produced by setup-file-length-check.sh and invoke the checker as "bash
scripts/check-file-length.sh || exit 1" (ensure the script path matches the
project convention: scripts/check-file-length.sh) so the hook logs when it runs
and fails the commit on violations.
- Line 319: The SCRIPT_DEST variable is pointing to the wrong path
("script/check-file-length.sh") causing the pre-commit hook to reference a
non-existent file; change SCRIPT_DEST to "scripts/check-file-length.sh" and
update any hook append/invocation that adds the script to the pre-commit hook so
it calls scripts/check-file-length.sh (the same path used by
setup-file-length-check.sh), ensuring all references (the variable SCRIPT_DEST
and the pre-commit hook append/invocation) are consistent.
- Line 357: The comment "すでに lint-staged を呼んでいる行の後に追加" claims insertion after
the lint-staged invocation but the current logic just appends to the pre-commit
file; either update that comment to state "append to end of pre-commit" or
implement intelligent insertion: detect the last occurrence of "lint-staged" in
the pre-commit file (grep -q "lint-staged" or similar) and use sed to insert the
block (the check-file-length echo and bash scripts/check-file-length.sh || exit
1 lines) after that line, falling back to appending if no match is found; refer
to the "lint-staged" string and the inserted block (check-file-length /
scripts/check-file-length.sh) when making the change.
---
Outside diff comments:
In @.claude/commands/repo-maintenance.md:
- Around line 1-5: The PR is missing a required linked issue; update the PR
description to include a "Linked Issue" line with the issue URL or number (e.g.
"Linked Issue: `#123`" or "Linked Issue: https://...") or create the issue first
and then add its link; if creating a new issue, open one with a clear title and
brief description summarizing the change and then paste that issue link into the
PR body (ensure the PR body contains exactly "Linked Issue: <issue-url-or-#>" so
the repo's quality gate recognizes it).
---
Nitpick comments:
In @.claude/commands/repo-maintenance.md:
- Around line 366-377: The .filelengthignore creation uses a hardcoded
IGNORE_TEMPLATE path and a minimal inline default; update this to follow the
existing script/setup-file-length-check.sh logic by resolving the template via
the same TEMPLATE_DIR (or same resolution function) instead of
/usr/local/share/config-templates, and either replace the inline heredoc for
.filelengthignore with the comprehensive default patterns from
script/setup-file-length-check.sh or copy the canonical template file from
TEMPLATE_DIR/.filelengthignore.template when present; adjust references to
IGNORE_TEMPLATE and the .filelengthignore heredoc accordingly so the two places
stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a89d2f0d-e104-4cb1-af69-7ac1154f4340
📒 Files selected for processing (1)
.claude/commands/repo-maintenance.md
|
|
||
| ```bash | ||
| PRECOMMIT=".husky/pre-commit" | ||
| SCRIPT_DEST="script/check-file-length.sh" |
There was a problem hiding this comment.
Critical: Path mismatch with existing setup script.
Line 319 specifies script/check-file-length.sh (singular), and line 360 appends this path to the pre-commit hook. However, the existing script/setup-file-length-check.sh copies the script to scripts/check-file-length.sh (plural, see context snippet 1 lines 24-36). This inconsistency means:
- If a repository was previously set up using the automated setup script, it has the script at
scripts/check-file-length.sh - If this repo-maintenance logic runs, it will add a hook calling
script/check-file-length.sh - The hook will fail because the script is at a different path
🔧 Proposed fix
Align with the existing setup script by using scripts/ (plural):
-SCRIPT_DEST="script/check-file-length.sh"
+SCRIPT_DEST="scripts/check-file-length.sh"And update the hook invocation:
-echo "bash script/check-file-length.sh" >> "$PRECOMMIT"
+echo "bash scripts/check-file-length.sh" >> "$PRECOMMIT"Also update line 343:
-mkdir -p script
+mkdir -p scriptsBased on relevant code snippet from script/setup-file-length-check.sh:24-36 showing the existing setup script uses "scripts/" directory.
Also applies to: 360-360
🤖 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 319, The SCRIPT_DEST variable
is pointing to the wrong path ("script/check-file-length.sh") causing the
pre-commit hook to reference a non-existent file; change SCRIPT_DEST to
"scripts/check-file-length.sh" and update any hook append/invocation that adds
the script to the pre-commit hook so it calls scripts/check-file-length.sh (the
same path used by setup-file-length-check.sh), ensuring all references (the
variable SCRIPT_DEST and the pre-commit hook append/invocation) are consistent.
| 2. pre-commit フックに追記: | ||
|
|
||
| ```bash | ||
| # すでに lint-staged を呼んでいる行の後に追加 |
There was a problem hiding this comment.
Minor: Comment doesn't match implementation.
Line 357's comment states "すでに lint-staged を呼んでいる行の後に追加" (add after the line that calls lint-staged), but the actual code on lines 358-360 simply appends to the end of the pre-commit file without any logic to locate or insert after the lint-staged invocation. Either implement intelligent insertion or update the comment to reflect the actual behavior.
♻️ Option 1: Update comment to match implementation
-# すでに lint-staged を呼んでいる行の後に追加
+# pre-commit の末尾に追加♻️ Option 2: Implement intelligent insertion
# Insert after lint-staged line if it exists, otherwise append to end
if grep -q "lint-staged" "$PRECOMMIT" 2>/dev/null; then
# Insert after the last lint-staged line
sed -i '/lint-staged/a\\n# check-file-length\necho "[husky] Running file length check..."\nbash scripts/check-file-length.sh || exit 1' "$PRECOMMIT"
else
# Append to end
...
fi🤖 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 357, The comment "すでに
lint-staged を呼んでいる行の後に追加" claims insertion after the lint-staged invocation but
the current logic just appends to the pre-commit file; either update that
comment to state "append to end of pre-commit" or implement intelligent
insertion: detect the last occurrence of "lint-staged" in the pre-commit file
(grep -q "lint-staged" or similar) and use sed to insert the block (the
check-file-length echo and bash scripts/check-file-length.sh || exit 1 lines)
after that line, falling back to appending if no match is found; refer to the
"lint-staged" string and the inserted block (check-file-length /
scripts/check-file-length.sh) when making the change.
| echo "" >> "$PRECOMMIT" | ||
| echo "# check-file-length" >> "$PRECOMMIT" | ||
| echo "bash script/check-file-length.sh" >> "$PRECOMMIT" |
There was a problem hiding this comment.
Major: Hook invocation format inconsistent with existing setup script.
Lines 358-360 append a hook invocation without error handling (|| exit 1) or logging. The existing script/setup-file-length-check.sh appends THREE lines including an echo statement for logging and || exit 1 to ensure the pre-commit fails if the check fails (see context snippet 2 lines 65-80):
echo "echo \"[husky] Running file length check...\""
echo "bash scripts/check-file-length.sh || exit 1"Without || exit 1, the pre-commit hook will continue even if file length violations are detected, defeating the purpose of the check.
🔧 Proposed fix to align with existing setup script
-echo "" >> "$PRECOMMIT"
-echo "# check-file-length" >> "$PRECOMMIT"
-echo "bash script/check-file-length.sh" >> "$PRECOMMIT"
+{
+ echo ""
+ echo "echo \"[husky] Running file length check...\""
+ echo "bash scripts/check-file-length.sh || exit 1"
+} >> "$PRECOMMIT"Based on relevant code snippet from script/setup-file-length-check.sh:65-80 showing the ACTUAL hook format produced by the existing setup script.
🤖 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 358 - 360, The pre-commit
hook lines being appended (using PRECOMMIT and "bash
script/check-file-length.sh") lack the logging echo and the "|| exit 1" failure
propagation used by script/setup-file-length-check.sh; update the invocation to
match the existing pattern by appending an echo logging line like the one
produced by setup-file-length-check.sh and invoke the checker as "bash
scripts/check-file-length.sh || exit 1" (ensure the script path matches the
project convention: scripts/check-file-length.sh) so the hook logs when it runs
and fails the commit on violations.
|
PR レビュー: feat: repo-maintenanceにcheck-file-length自動追加を組み込む 全体的に機能の意図は明確で、既存のパターンとも一貫しています。 良い点
指摘事項
テストプランへの補足提案
全体として品質ゲートの自動化という目的は明確で有用な変更です。指摘 1 と 2 を対応いただけると、より安定した動作が期待できます。 Reviewed by Claude Sonnet 4.6 |
|
🎉 This PR is included in version 1.86.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
/repo-maintenance実行時に、huskyが設定済みだがcheck-file-lengthが未設定のリポジトリへ自動追加するStep 3.2.1を追加.filelengthignoreがなければデフォルトテンプレートを自動生成Test plan
/repo-maintenanceを実行し、check-file-lengthが pre-commit に追加されることを確認check-file-lengthが設定済みのリポジトリでは ✅ スキップされることを確認🤖 Generated with Claude Code
Summary by CodeRabbit