Skip to content

feat: ファイル行数チェック機能を追加 - #484

Merged
keito4 merged 2 commits into
mainfrom
feat/file-length-check
Feb 19, 2026
Merged

feat: ファイル行数チェック機能を追加#484
keito4 merged 2 commits into
mainfrom
feat/file-length-check

Conversation

@keito4

@keito4 keito4 commented Feb 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • DevContainer環境で使用可能なファイル行数チェック機能を追加
  • 350行以上で警告、500行以上でコミットをブロック
  • .filelengthignore で除外パターンを設定可能
  • setup-husky スキルに filelength チェックを統合
  • skills.txtreact-doctor を追加

追加ファイル

ファイル 役割
script/check-file-length.sh 行数チェックスクリプト
script/setup-file-length-check.sh セットアップスクリプト
.devcontainer/templates/.filelengthignore.template 除外パターンテンプレート

他のリポジトリでの使用方法

# DevContainer 環境
/usr/local/script/setup-file-length-check.sh

# または手動
mkdir -p scripts
cp /usr/local/script/check-file-length.sh scripts/
cp /usr/local/share/config-templates/.filelengthignore.template .filelengthignore

Test plan

  • DevContainer ビルドが成功すること
  • check-file-length.sh が正常に動作すること
  • setup-file-length-check.sh でセットアップできること

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Integrated file-length validation into the development workflow with configurable thresholds (warn at 350 lines, block at 500 lines).
    • Added automation to install and wire up the file-length check and ignore template during setup.
    • Added 1Password CLI to the development container environment.
    • Added react-doctor to the React skills toolset and several new skill references.
  • Documentation

    • Updated setup guidance and templates to document multi-part Husky configuration, ignore patterns, and file-length settings.

DevContainer環境で使用可能なファイル行数チェック機能を追加しました。

- script/check-file-length.sh: 行数チェックスクリプト
  - 350行以上で警告、500行以上でコミットをブロック
  - .filelengthignore で除外パターンを設定可能
- script/setup-file-length-check.sh: セットアップスクリプト
- .devcontainer/templates/.filelengthignore.template: 除外パターンテンプレート
- setup-husky スキルに filelength チェックを統合
- skills.txt に react-doctor を追加

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Feb 19, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds file-length enforcement to the dev workflow (warn at 350 lines, block at 500), including scripts and templates, integrates the check into Husky pre-commit, updates DevContainer to provide templates and installs 1Password CLI, and adds small skills registry entries.

Changes

Cohort / File(s) Summary
Husky & file-length scripts
.claude/commands/setup-husky.md, script/setup-file-length-check.sh, script/check-file-length.sh
Adds setup automation and a runtime checker; integrates file-length check into Husky pre-commit, uses env vars for thresholds, robust git/staged-file handling, and supports .filelengthignore.
File-length ignore template
.devcontainer/templates/.filelengthignore.template
New .filelengthignore template with examples and common exclusion patterns (generated files, build outputs, tests, lockfiles).
DevContainer image & templates
.devcontainer/Dockerfile, .devcontainer/templates/...
Installs unzip and 1Password CLI (op) in the container and copies project templates into /usr/local/share/config-templates/ for use by setup scripts.
Skills registry entries
.claude/skills/skills.txt, .claude/skills/...
Adds millionco/react-doctor to React skills and creates small skill pointer files.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer
  participant Git as Git (commit)
  participant Husky as Husky pre-commit
  participant Script as check-file-length.sh
  participant Repo as Repo (.filelengthignore)

  Dev->>Git: git commit (staged)
  Git->>Husky: trigger pre-commit
  Husky->>Script: run file-length check on staged files
  Script->>Repo: git check-ignore / read .filelengthignore
  Script->>Husky: return pass/warn/fail
  Husky->>Git: allow or abort commit (with messages)
  Note over Dev,Script: WARN at >=350 lines, FAIL at >=500 lines (env-configurable)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

released, size/S

Poem

🐰 I hopped through scripts and templates bright,
I nudged the hook to keep files light,
Warn at three-fifty, stop at five-hundred true,
A tidy repo — carrot-cheers to you! 🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title in Japanese translates to 'feat: Add file line count check feature', which accurately describes the main changeset focusing on file-length checks and integration into the setup.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/file-length-check

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c86826ce3d

ℹ️ 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".

Comment on lines +40 to +41
if [ -f "$TEMPLATE_DIR/.filelengthignore.template" ]; then
cp "$TEMPLATE_DIR/.filelengthignore.template" .filelengthignore

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Read repo template before writing fallback ignore list

setup-file-length-check.sh only looks for .filelengthignore.template under /usr/local/share/config-templates and otherwise writes a reduced hardcoded fallback, so running the script directly from this repo (or any non-devcontainer environment) produces different ignore rules than the bundled template and can unexpectedly block commits for files that should be excluded. Please check a local bundled template path before falling back to the minimal inline content.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (4)
.devcontainer/templates/.filelengthignore.template (1)

29-31: Redundant lockfile patterns; yarn.lock also missing.

Since staged_files() in check-file-length.sh only collects *.ts, *.tsx, *.js, *.jsx files, package-lock.json and pnpm-lock.yaml patterns are dead entries — they'll never reach the ignore check. Removing them avoids misleading users about the tool's scope. If lockfile coverage is intended for future extension of the script, yarn.lock should be added for completeness.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.devcontainer/templates/.filelengthignore.template around lines 29 - 31, The
listed lockfile ignore patterns (**/package-lock.json and **/pnpm-lock.yaml) are
dead entries given staged_files() in check-file-length.sh only collects *.ts,
*.tsx, *.js, *.jsx; remove those two patterns from .filelengthignore.template to
avoid confusion, and if you intend to extend check-file-length.sh to include
lockfiles later, either add **/yarn.lock now for completeness or add a TODO
comment referencing staged_files() in check-file-length.sh so maintainers know
why lockfiles are absent.
script/check-file-length.sh (1)

11-12: No validation that FILE_LENGTH_HARD_LIMIT / FILE_LENGTH_WARN_LIMIT are integers.

If a consumer sets FILE_LENGTH_HARD_LIMIT=abc, the -ge comparisons at lines 39 and 41 will emit bash: [: abc: integer expression expected and exit with a non-zero code, which set -e then escalates to an abort. A quick guard prevents a confusing failure:

🔧 Proposed fix
 HARD_LIMIT=${FILE_LENGTH_HARD_LIMIT:-500}
 WARN_LIMIT=${FILE_LENGTH_WARN_LIMIT:-350}
+if ! [[ "$HARD_LIMIT" =~ ^[0-9]+$ ]] || ! [[ "$WARN_LIMIT" =~ ^[0-9]+$ ]]; then
+  echo "[file-length] ERROR: FILE_LENGTH_HARD_LIMIT and FILE_LENGTH_WARN_LIMIT must be positive integers" >&2
+  exit 1
+fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/check-file-length.sh` around lines 11 - 12, HARD_LIMIT and WARN_LIMIT
are not validated as integers, so non-numeric env values
(FILE_LENGTH_HARD_LIMIT/FILE_LENGTH_WARN_LIMIT) cause the numeric comparisons
later to fail; add validation right after the HARD_LIMIT/WARN_LIMIT assignments
to ensure each value matches an integer regex (e.g. ^[0-9]+$), and if a value is
not a valid integer, print a clear error and exit non-zero; update the script
symbols HARD_LIMIT, WARN_LIMIT and the environment variables
FILE_LENGTH_HARD_LIMIT/FILE_LENGTH_WARN_LIMIT so the numeric checks before the
-ge comparisons reject non-integers.
.devcontainer/Dockerfile (1)

51-51: unzip -od /usr/local/bin/ may scatter extra files into /usr/local/bin/.

The -o flag overwrites without prompting and -d /usr/local/bin/ dumps every file in the archive into a critical $PATH directory. If a future release of the zip packages additional files (signatures, READMEs, etc.), they land in /usr/local/bin/. Extracting to a scratch directory and moving only op is safer:

🔧 Proposed fix
- && unzip -od /usr/local/bin/ /tmp/op.zip \
+ && unzip -o /tmp/op.zip op -d /usr/local/bin/ \

(The op positional argument tells unzip to extract only the file named op from the archive.)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.devcontainer/Dockerfile at line 51, The Dockerfile currently uses "unzip
-od /usr/local/bin/ /tmp/op.zip" which extracts every archive member into
/usr/local/bin; instead extract only the "op" member into a scratch directory
and then move that single file into /usr/local/bin to avoid dropping unexpected
files into PATH. Update the command that references "unzip -od /usr/local/bin/
/tmp/op.zip" to extract only the "op" entry from "/tmp/op.zip" into a temporary
directory (or use unzip’s member-selection), then mv the extracted "op" binary
into /usr/local/bin and set executable permissions (chmod +x) on "op".
script/setup-file-length-check.sh (1)

67-67: Duplicate-detection grep also matches commented-out occurrences.

grep -q "check-file-length.sh" matches # bash scripts/check-file-length.sh (a commented-out entry), so the hook would not be added when it's actually inactive. Anchoring to the executable line prevents the false positive:

🔧 Proposed fix
-  if ! grep -q "check-file-length.sh" .husky/pre-commit; then
+  if ! grep -qE "^[^#]*check-file-length\.sh" .husky/pre-commit; then
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/setup-file-length-check.sh` at line 67, The current
duplicate-detection grep in setup-file-length-check.sh falsely matches commented
lines in .husky/pre-commit (e.g., "# bash scripts/check-file-length.sh"); update
the grep used when checking for "check-file-length.sh" in .husky/pre-commit so
it only matches the actual executable hook line (anchor to the line start and
optional whitespace and the "bash scripts/check-file-length.sh" invocation or
require the line not start with '#'), replacing the simple unanchored search to
avoid skipping adding the hook when the entry is commented out.
🤖 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/setup-husky.md:
- Around line 29-31: The snippet that copies check-file-length.sh doesn't make
the file executable; after the cp in the conditional block that copies
/usr/local/script/check-file-length.sh to scripts/, run an explicit chmod +x on
scripts/check-file-length.sh (matching the approach used in
setup-file-length-check.sh) so the pre-commit hook can execute without
permission errors.
- Around line 153-156: The markdown entry `**/_.generated._` is being rendered
incorrectly due to Markdown treating `*` and `_` as emphasis; update the example
so the glob is shown literally—either wrap the snippet in a fenced code block or
escape the asterisks/underscores (e.g., replace with `**/*.generated.*` or
escape characters) to ensure the `.filelengthignore` pattern copies correctly;
edit the text containing the glob pattern (`**/_.generated._`) to use a fenced
code block or the escaped form.

In @.devcontainer/Dockerfile:
- Around line 48-55: Update OP_VERSION to "2.32.1" and add signature/integrity
verification for the downloaded 1Password CLI before unpacking and changing
ownership/permissions: keep the curl step but also download the vendor-provided
signature and public key, verify the ZIP with gpg (or another recommended
1Password verification flow) and abort the build if verification fails, and only
then unzip, remove the temp files, run groupadd onepassword-cli, chown
root:onepassword-cli on the installed binary and apply chmod g+s; ensure the
verification step is implemented around the existing OP_VERSION variable and the
curl/unzip sequence so the build fails on integrity check failure.

In `@script/check-file-length.sh`:
- Line 37: The pipeline that computes line_count using "git show ":$file" | wc
-l" will cause the script to abort under set -euo pipefail when git show returns
non-zero; modify the command that defines line_count in check-file-length.sh
(the line assigning to line_count and referencing $file) to append "|| continue"
so that a failing git show is treated as a skip for that file instead of killing
the script, ensuring the loop continues on problematic files.

In `@script/setup-file-length-check.sh`:
- Around line 68-70: Replace the three separate appends that each end with ">>
.husky/pre-commit" (the three echo lines that write the empty line, the husky
message, and the bash invocation) with a single grouped redirect: write the
three echo commands into one block and append that block once to
.husky/pre-commit so ShellCheck SC2129 is satisfied.

---

Nitpick comments:
In @.devcontainer/Dockerfile:
- Line 51: The Dockerfile currently uses "unzip -od /usr/local/bin/ /tmp/op.zip"
which extracts every archive member into /usr/local/bin; instead extract only
the "op" member into a scratch directory and then move that single file into
/usr/local/bin to avoid dropping unexpected files into PATH. Update the command
that references "unzip -od /usr/local/bin/ /tmp/op.zip" to extract only the "op"
entry from "/tmp/op.zip" into a temporary directory (or use unzip’s
member-selection), then mv the extracted "op" binary into /usr/local/bin and set
executable permissions (chmod +x) on "op".

In @.devcontainer/templates/.filelengthignore.template:
- Around line 29-31: The listed lockfile ignore patterns (**/package-lock.json
and **/pnpm-lock.yaml) are dead entries given staged_files() in
check-file-length.sh only collects *.ts, *.tsx, *.js, *.jsx; remove those two
patterns from .filelengthignore.template to avoid confusion, and if you intend
to extend check-file-length.sh to include lockfiles later, either add
**/yarn.lock now for completeness or add a TODO comment referencing
staged_files() in check-file-length.sh so maintainers know why lockfiles are
absent.

In `@script/check-file-length.sh`:
- Around line 11-12: HARD_LIMIT and WARN_LIMIT are not validated as integers, so
non-numeric env values (FILE_LENGTH_HARD_LIMIT/FILE_LENGTH_WARN_LIMIT) cause the
numeric comparisons later to fail; add validation right after the
HARD_LIMIT/WARN_LIMIT assignments to ensure each value matches an integer regex
(e.g. ^[0-9]+$), and if a value is not a valid integer, print a clear error and
exit non-zero; update the script symbols HARD_LIMIT, WARN_LIMIT and the
environment variables FILE_LENGTH_HARD_LIMIT/FILE_LENGTH_WARN_LIMIT so the
numeric checks before the -ge comparisons reject non-integers.

In `@script/setup-file-length-check.sh`:
- Line 67: The current duplicate-detection grep in setup-file-length-check.sh
falsely matches commented lines in .husky/pre-commit (e.g., "# bash
scripts/check-file-length.sh"); update the grep used when checking for
"check-file-length.sh" in .husky/pre-commit so it only matches the actual
executable hook line (anchor to the line start and optional whitespace and the
"bash scripts/check-file-length.sh" invocation or require the line not start
with '#'), replacing the simple unanchored search to avoid skipping adding the
hook when the entry is commented out.

Comment on lines +29 to +31
if [ -f /usr/local/script/check-file-length.sh ]; then
cp /usr/local/script/check-file-length.sh scripts/
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing chmod +x after copying check-file-length.sh.

cp does not reliably preserve the execute bit (it is subject to the process's umask). Without an explicit chmod +x, the copied script may not be executable, causing the pre-commit hook to fail with Permission denied. The corresponding automated path in setup-file-length-check.sh does include chmod +x scripts/check-file-length.sh; the manual snippet should match:

🔧 Proposed fix
 if [ -f /usr/local/script/check-file-length.sh ]; then
 cp /usr/local/script/check-file-length.sh scripts/
+chmod +x scripts/check-file-length.sh
 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.

Suggested change
if [ -f /usr/local/script/check-file-length.sh ]; then
cp /usr/local/script/check-file-length.sh scripts/
fi
if [ -f /usr/local/script/check-file-length.sh ]; then
cp /usr/local/script/check-file-length.sh scripts/
chmod +x scripts/check-file-length.sh
fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/setup-husky.md around lines 29 - 31, The snippet that
copies check-file-length.sh doesn't make the file executable; after the cp in
the conditional block that copies /usr/local/script/check-file-length.sh to
scripts/, run an explicit chmod +x on scripts/check-file-length.sh (matching the
approach used in setup-file-length-check.sh) so the pre-commit hook can execute
without permission errors.

Comment on lines +153 to +156
# 自動生成ファイル

**/_.generated._
**/database.types.ts

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Glob pattern corrupted by Markdown italic rendering.

**/_.generated._ will render as **/ followed by italic _generated_ followed by nothing — the * delimiters are consumed by the Markdown parser. Users who copy this pattern will get a broken .filelengthignore entry. Wrap the example block in a fenced code block, or at minimum escape the asterisks:

🔧 Proposed fix
-**/_.generated._
+**/*.generated.*

Or wrap the entire example in a fenced code block to prevent Markdown interpretation:

# 自動生成ファイル
**/*.generated.*
**/database.types.ts
📝 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
# 自動生成ファイル
**/_.generated._
**/database.types.ts
# 自動生成ファイル
**/*.generated.*
**/database.types.ts
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/setup-husky.md around lines 153 - 156, The markdown entry
`**/_.generated._` is being rendered incorrectly due to Markdown treating `*`
and `_` as emphasis; update the example so the glob is shown literally—either
wrap the snippet in a fenced code block or escape the asterisks/underscores
(e.g., replace with `**/*.generated.*` or escape characters) to ensure the
`.filelengthignore` pattern copies correctly; edit the text containing the glob
pattern (`**/_.generated._`) to use a fenced code block or the escaped form.

Comment thread .devcontainer/Dockerfile Outdated
Comment on lines +48 to +55
RUN OP_VERSION="2.30.3" \
&& ARCH=$(dpkg --print-architecture) \
&& curl -sSfo /tmp/op.zip "https://cache.agilebits.com/dist/1P/op2/pkg/v${OP_VERSION}/op_linux_${ARCH}_v${OP_VERSION}.zip" \
&& unzip -od /usr/local/bin/ /tmp/op.zip \
&& rm /tmp/op.zip \
&& groupadd -f onepassword-cli \
&& chown root:onepassword-cli /usr/local/bin/op \
&& chmod g+s /usr/local/bin/op

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

1Password CLI op 2 latest stable version 2026 releases

💡 Result:

Latest stable 1Password CLI v2 (op): 2.32.1 — released February 4, 2026 (release notes) / February 5, 2026 (developer releases page). (app-updates.agilebits.com)

2026 stable releases (so far):

Citations:


🌐 Web query:

1Password CLI 2.32.1 SHA256 checksum Linux amd64 official

💡 Result:

I couldn’t find any officially published SHA-256 checksum from 1Password/AgileBits for the standalone 1Password CLI 2.32.1 Linux amd64 ZIP (the download is listed, but no checksum is provided alongside it). [1]

For reference, the official download for Linux amd64 in v2.32.1 is: op_linux_amd64_v2.32.1.zip (released 2026-02-04). [1]

1Password’s official guidance for “authenticity” focuses on signature / installer verification rather than publishing per-file SHA-256 sums for manual comparison. [2]

Sources:
[1] (app-updates.agilebits.com)
[2] (support.1password.com)

Citations:


Update 1Password CLI to 2.32.1 and add integrity verification.

The binary is fetched from the CDN and unpacked directly without any integrity check. Version 2.30.3 is outdated — the latest stable release is 2.32.1 (Feb 4, 2026). A compromised CDN response or MitM attack would silently install a malicious op binary that is then g+s-promoted, escalating the blast radius.

1Password does not publish SHA256 checksums for standalone CLI ZIP downloads. Instead, verify the download using their signature verification process per their security documentation, or pin to 2.32.1 and document the verification method your deployment uses.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.devcontainer/Dockerfile around lines 48 - 55, Update OP_VERSION to "2.32.1"
and add signature/integrity verification for the downloaded 1Password CLI before
unpacking and changing ownership/permissions: keep the curl step but also
download the vendor-provided signature and public key, verify the ZIP with gpg
(or another recommended 1Password verification flow) and abort the build if
verification fails, and only then unzip, remove the temp files, run groupadd
onepassword-cli, chown root:onepassword-cli on the installed binary and apply
chmod g+s; ensure the verification step is implemented around the existing
OP_VERSION variable and the curl/unzip sequence so the build fails on integrity
check failure.

is_ignored "$file" && continue

line_count=$(git show ":$file" 2>/dev/null | wc -l)
line_count=$(git show ":$file" 2>/dev/null | wc -l | tr -d ' ')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

set -euo pipefail + failed git show will abort the hook and falsely block commits.

With pipefail active, if git show ":$file" exits non-zero (e.g. for a binary file, an encoding edge case, or a transient git issue), the pipeline's exit code propagates to the assignment, which—at statement level inside the loop—triggers set -e and kills the script. The commit is then blocked with no meaningful error message.

Adding || continue converts a fatal abort into a graceful skip for the problematic file:

🔧 Proposed fix
-  line_count=$(git show ":$file" 2>/dev/null | wc -l | tr -d ' ')
+  line_count=$(git show ":$file" 2>/dev/null | wc -l | tr -d ' ') || continue
📝 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
line_count=$(git show ":$file" 2>/dev/null | wc -l | tr -d ' ')
line_count=$(git show ":$file" 2>/dev/null | wc -l | tr -d ' ') || continue
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/check-file-length.sh` at line 37, The pipeline that computes
line_count using "git show ":$file" | wc -l" will cause the script to abort
under set -euo pipefail when git show returns non-zero; modify the command that
defines line_count in check-file-length.sh (the line assigning to line_count and
referencing $file) to append "|| continue" so that a failing git show is treated
as a skip for that file instead of killing the script, ensuring the loop
continues on problematic files.

Comment thread script/setup-file-length-check.sh Outdated
- script/setup-file-length-check.sh: 複数リダイレクトをグループ化(SC2129)
- Dockerfile: 1Password CLI を 2.30.3 → 2.32.1 にアップデート(CVE-2024-45337修正)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Feb 19, 2026

Copy link
Copy Markdown
Contributor

コードレビュー

全体的な方向性は良好です。以下、気になった点を指摘します。


🔴 要修正

1. setup-file-length-check.sh — スクリプトコピー戦略の保守性問題

各プロジェクトに check-file-length.sh をコピーする設計は、スクリプトが更新された際に全プロジェクトへの再配布が必要になり保守負債が生じます。

コピー戦略を採用するなら、バージョン情報をファイルに埋め込んで「このコピーがいつ何のバージョンか」が追跡できるようにすべきです。DevContainer 環境では /usr/local/script/check-file-length.sh を直接参照する方式も検討の余地があります。

2. Dockerfile — 1Password CLI のチェックサム未検証

ダウンロード後にチェックサム検証がありません。Doppler のインストール部分は .deb のシグネチャ検証が apt にあるため安全ですが、zipファイルは公式の SHA256 ハッシュと照合すべきです。

# 改善例
&& echo "EXPECTED_SHA256  /tmp/op.zip" | sha256sum -c - \

3. setup-husky.md — ドキュメント内のパターンが文字化け

Markdown のイタリック記法と衝突して *.generated.*_.generated._ と表示されています。コードブロックで囲む必要があります。


🟡 改善推奨

4. .filelengthignore.template — テストファイルの除外はTDD方針と矛盾

CLAUDE.md の TDD 方針(70%+ カバレッジ、クリティカルパス100%)を考えると、**/*.test.ts**/*.spec.ts をデフォルト除外することでテストの肥大化を見逃す可能性があります。テストファイルも行数制限の対象に含めることを検討してください。

5. check-file-length.sh — git 管理外での動作が暗黙的

REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" は git 管理外ディレクトリでは pwd にフォールバックしますが、その場合 staged_files() は空を返し、何もチェックしないまま正常終了します。意図的な動作であればコメントで明記してください。

6. 新規シンボリックリンクと skills.txt の不整合

.claude/skills/find-skills.claude/skills/supabase-postgres-best-practices のシンボリックリンクが追加されましたが、skills.txt に対応エントリがありません。これらはどこで管理・インストールされる想定でしょうか?


🟢 良い点

  • wc -l | tr -d ' ' の追加は macOS 互換性の修正として適切
  • FILE_LENGTH_HARD_LIMIT / FILE_LENGTH_WARN_LIMIT の環境変数化で柔軟性が向上
  • staged_files() への 2>/dev/null || true 追加でCI環境での堅牢性が向上
  • setup-file-length-check.sh の冪等性確認(.filelengthignore 既存チェック、Huskyへの重複追加防止)は丁寧な実装

テスト計画について

PR説明のテスト計画がすべて未チェックです。CLAUDE.md の Definition of Done では「すべての Quality Gate Pass」が必須です。手動確認済みであればチェックを入れてください。

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
script/setup-file-length-check.sh (2)

12-19: Optional: validate TARGET_DIR existence before cd.

With set -euo pipefail, a missing TARGET_DIR causes the script to exit on cd with Bash's own terse error, which may be confusing to users.

♻️ Proposed improvement
 TARGET_DIR="${1:-.}"
+if [ ! -d "$TARGET_DIR" ]; then
+  echo "[ERROR] ターゲットディレクトリが見つかりません: $TARGET_DIR"
+  exit 1
+fi
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/setup-file-length-check.sh` around lines 12 - 19, Check that
TARGET_DIR exists and is a directory before calling cd "$TARGET_DIR": verify the
variable (TARGET_DIR) is non-empty and that [ -d "$TARGET_DIR" ] holds, and if
not emit a clear error via echo to stderr and exit with a non-zero status; place
this validation immediately before the cd "$TARGET_DIR" line so the script fails
with a friendly message instead of Bash's terse error when using set -euo
pipefail.

24-36: Minor: deduplicate copy logic with a source-path variable.

The cp / chmod / echo trio is identical in both branches — only the source path differs.

♻️ Proposed refactor
-# check-file-length.sh をコピー
-if [ -f "$SCRIPT_DIR/check-file-length.sh" ]; then
-  cp "$SCRIPT_DIR/check-file-length.sh" scripts/
-  chmod +x scripts/check-file-length.sh
-  echo "[SUCCESS] scripts/check-file-length.sh をコピーしました"
-elif [ -f /usr/local/script/check-file-length.sh ]; then
-  cp /usr/local/script/check-file-length.sh scripts/
-  chmod +x scripts/check-file-length.sh
-  echo "[SUCCESS] scripts/check-file-length.sh をコピーしました"
-else
+# check-file-length.sh をコピー
+_src=""
+if [ -f "$SCRIPT_DIR/check-file-length.sh" ]; then
+  _src="$SCRIPT_DIR/check-file-length.sh"
+elif [ -f /usr/local/script/check-file-length.sh ]; then
+  _src="/usr/local/script/check-file-length.sh"
+fi
+
+if [ -n "$_src" ]; then
+  cp "$_src" scripts/
+  chmod +x scripts/check-file-length.sh
+  echo "[SUCCESS] scripts/check-file-length.sh をコピーしました"
+else
   echo "[ERROR] check-file-length.sh が見つかりません"
   exit 1
 fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/setup-file-length-check.sh` around lines 24 - 36, Detect the source
path once into a variable (e.g. SOURCE) by testing if
"$SCRIPT_DIR/check-file-length.sh" exists, else if
"/usr/local/script/check-file-length.sh" exists, otherwise exit with error; then
perform the duplicate operations (cp "$SOURCE" scripts/, chmod +x
scripts/check-file-length.sh, echo success) exactly once. Update the block
around the checks for SCRIPT_DIR and /usr/local/script so only the path is
chosen, and move the cp/chmod/echo trio to run after SOURCE is set (and avoid
running them in both branches).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@script/setup-file-length-check.sh`:
- Around line 65-76: The grouped redirect block using "{ ... } >>
.husky/pre-commit" correctly addresses the ShellCheck SC2129 warning for the
pre-commit hook modification; keep the block that appends the three echo lines
(the echo "", echo "[husky] Running file length check...", and echo "bash
scripts/check-file-length.sh || exit 1") guarded by the if [ -f
.husky/pre-commit ] and the conditional that checks grep -q
"check-file-length.sh", ensuring the grouped redirect remains as-is to avoid
SC2129 reoccurrence.
- Around line 78-79: Update the guidance for creating the .husky/pre-commit hook
in scripts/check-file-length.sh to remove the deprecated "npx husky add"
instruction and instead show the manual v9+ approach: instruct users to create
the .husky/pre-commit file containing the command "bash
scripts/check-file-length.sh" (or the equivalent shell wrapper) and then run
"chmod +x .husky/pre-commit" to make it executable; reference the
.husky/pre-commit path and the scripts/check-file-length.sh script so reviewers
can locate and update the lines accordingly.

---

Nitpick comments:
In `@script/setup-file-length-check.sh`:
- Around line 12-19: Check that TARGET_DIR exists and is a directory before
calling cd "$TARGET_DIR": verify the variable (TARGET_DIR) is non-empty and that
[ -d "$TARGET_DIR" ] holds, and if not emit a clear error via echo to stderr and
exit with a non-zero status; place this validation immediately before the cd
"$TARGET_DIR" line so the script fails with a friendly message instead of Bash's
terse error when using set -euo pipefail.
- Around line 24-36: Detect the source path once into a variable (e.g. SOURCE)
by testing if "$SCRIPT_DIR/check-file-length.sh" exists, else if
"/usr/local/script/check-file-length.sh" exists, otherwise exit with error; then
perform the duplicate operations (cp "$SOURCE" scripts/, chmod +x
scripts/check-file-length.sh, echo success) exactly once. Update the block
around the checks for SCRIPT_DIR and /usr/local/script so only the path is
chosen, and move the cp/chmod/echo trio to run after SOURCE is set (and avoid
running them in both branches).

@keito4
keito4 merged commit 1335367 into main Feb 19, 2026
20 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.78.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released リリース済み

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant