Skip to content

chore(husky): pre-commit/pre-push を Docker/host 両対応にし重い解析を pre-push へ分離#6900

Open
ttokoro20240902 wants to merge 1 commit into
4.4from
feature/husky-hooks-docker-host
Open

chore(husky): pre-commit/pre-push を Docker/host 両対応にし重い解析を pre-push へ分離#6900
ttokoro20240902 wants to merge 1 commit into
4.4from
feature/husky-hooks-docker-host

Conversation

@ttokoro20240902

@ttokoro20240902 ttokoro20240902 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

概要(Overview・Refs Issue)

Git フック(husky)を Docker 環境・host 環境のどちらでも動くようにし、あわせて 重い静的解析を pre-commit から pre-push へ分離します。

docker-compose で開発している環境では、host に PHP / node を持たない構成が一般的です。現行および #6761 の hook は vendor/bin/* / npxhost 実行前提にしているため、そうした環境では hook が失敗、または握り潰し(|| node -e '')によって無音で素通りし「掛けているつもりで実は掛かっていない」状態になりがちでした。

Refs #6761(pre-commit 強化の提案。本 PR はその Docker/host 両対応版・別案です)

方針(Policy)

  • 実行環境を自動判定する RUN プレフィックスを導入:

    • ec-cube コンテナ稼働中 → docker compose exec -T ec-cube …
    • コンテナ無し & host に php + vendor/bin → host 実行
    • どちらも無い → 握り潰さず明示スキップcommand -v php まで見て「phar は在るが php ランタイム無し」の誤判定も回避)
    • ECCUBE_HOOK_RUNNER=docker|host|skip で強制切替、HUSKY=0 でバイパス
  • フックを役割で分離(commit は頻繁=軽く、push は「人に渡す境界」= CI と同じ役割):

    フック 中身 手元実測
    pre-commit php-cs-fixer(staged のみ auto-fix) 約 0.5 秒
    pre-push rector --dry-run + phpstan(src 全体) 初回 ~35 秒 / 以降 ~4.5 秒
  • npx lint-staged を直接 php-cs-fixer --path-mode=intersection + git add に置換:コンテナに node/lint-staged が無くても動くようにするため(host でも同じ経路)。

  • キャッシュ削除(rm -rf var/rector_cache / /tmp/phpstan)を廃止:commit/push ごとに消すと毎回コールドで遅い。incremental キャッシュを活かし、pre-push は 2 回目以降 ~4.5 秒に収まります。

実装に関する補足(Appendix)

  • husky v9 は npm install 時に .husky/_/ 配下へ全 git フックのラッパを生成するため、.husky/pre-push を追加するだけで push 時に発火します(追加設定不要)。
  • package.jsonlint-staged 設定は今回のフックからは呼ばれなくなります(残置しても無害。整理は follow-up 可)。
  • 変更は shell フック 2 本のみで、PHP・Twig・エンティティ等のプロダクトコードには一切触れていません。

テスト(Test)

手元の Docker 環境(ec-cube サービス)で end-to-end 確認済み:

  • pre-commit: staged .php 1 件 → php-cs-fixer 実行、約 0.5 秒で完了(exit 0)。.php 未 staged の commit は即素通り。
  • pre-push: rector --dry-run(OK)→ phpstan analyze src/No errors, 611 files)まで完走(exit 0)。初回 ~35 秒、キャッシュ維持で 2 回目以降 ~4.5 秒
  • 環境判定が running in Docker (ec-cube) に解決することを確認。ECCUBE_HOOK_RUNNER=host で host 実行にも切替可能(host に PHP 8.3 + vendor がある環境で確認)。

shell フックのため PHPUnit のテスト追加対象はありません。

相談(Discussion)

マイナーバージョン互換性保持のための制限事項チェックリスト

  • 既存機能の仕様変更はありません(開発用フックのみ・実行時挙動に影響なし)
  • フックポイントの呼び出しタイミングの変更はありません
  • フックポイントのパラメータの削除・データ型の変更はありません
  • twigファイルに渡しているパラメータの削除・データ型の変更はありません
  • Serviceクラスの公開関数の、引数の削除・データ型の変更はありません
  • 入出力ファイル(CSVなど)のフォーマット変更はありません

Summary by CodeRabbit

  • 新機能
    • コミット前にステージ済みの PHP ファイルを自動整形し、修正内容をそのまま再ステージするようになりました。
    • プッシュ前にコード整合性チェックと静的解析を自動実行するようになりました。
  • 改善
    • 実行環境を自動判定できるようになり、ローカル環境・コンテナ環境のどちらでも使いやすくなりました。
    • フックの無効化や対象ファイルなしの場合は、処理をすぐ終了するようになりました。

- 実行環境を自動判定(ec-cube コンテナ稼働中は `docker compose exec`、
  無ければ host の vendor/bin、どちらも無ければ握り潰さず明示スキップ)
- pre-commit: php-cs-fixer を staged のみ auto-fix(lint-staged 非依存=
  コンテナに node が無くても動作。約0.5秒)
- pre-push: rector --dry-run + phpstan を project-wide 実行(重い解析は
  「人に渡す境界」である push へ寄せ、commit のテンポを保つ)
- キャッシュ削除(rm -rf var/rector_cache /tmp/phpstan)を廃止し
  incremental キャッシュを活かす(pre-push 初回~35s→以降~4.5s)
- ECCUBE_HOOK_RUNNER=docker|host|skip で強制切替、HUSKY=0 でバイパス可

Refs #6761

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

.husky/pre-commit を、ステージ済み PHP ファイルに php-cs-fixer を適用する自動修正フックに変更し、.husky/pre-push を新規追加して rector と phpstan をプロジェクト全体で実行するようにした。両フックとも HUSKY=0 によるバイパスと ECCUBE_HOOK_RUNNER(docker/host/skip/auto)による実行環境判定を備える。

Changes

Husky フック更新

Layer / File(s) Summary
pre-commit: php-cs-fixer 自動修正
.husky/pre-commit
ステージ済み *.php を抽出し、ECCUBE_HOOK_RUNNER に応じて docker/host/skip/auto を判定、php-cs-fixer--path-mode=intersection で実行し、失敗時は終了コード 1、成功時は git add で再ステージする。
pre-push: rector と phpstan の検証
.husky/pre-push
新規追加。HUSKY=0 によるバイパスと ECCUBE_HOOK_RUNNER による実行環境判定を行い、rector process --dry-runphpstan analyze src/ を順に実行、失敗時は終了コード 1 で中断し、両方成功時に完了メッセージを出す。

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Dev as 開発者
  participant PreCommit as pre-commit
  participant PrePush as pre-push
  participant Tools as php-cs-fixer/rector/phpstan

  Dev->>PreCommit: git commit
  PreCommit->>Tools: php-cs-fixer 実行
  Tools-->>PreCommit: 結果
  PreCommit-->>Dev: 成功/失敗

  Dev->>PrePush: git push
  PrePush->>Tools: rector --dry-run, phpstan
  Tools-->>PrePush: 結果
  PrePush-->>Dev: 成功/失敗
Loading

Poem

コミットの前に毛づくろい、コードもピカピカ 🐰
プッシュの前にもう一度、静かにチェックするよ
rector と phpstan、耳をぴんと立てて
フックが守る、うさぎの畑
パチパチ跳ねて、今日も緑のテストへ 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Docker/host両対応化と重い解析をpre-pushへ分離した変更内容を適切に要約しています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/husky-hooks-docker-host

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.

@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: 4

🧹 Nitpick comments (4)
.husky/pre-commit (2)

1-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

package.jsonlint-staged 設定が未使用になっている可能性

package.json には同一の php-cs-fixer --config=.php-cs-fixer.dist.php --path-mode=intersection fix を呼ぶ lint-staged 設定が残っています。本PRで npx lint-staged の直接利用をシェルベースの実行に置き換えたのであれば、package.json 側の該当設定は死んだ設定として整理対象になり得ます(本ファイルの変更範囲外のため要確認)。

🤖 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 @.husky/pre-commit around lines 1 - 32, The package.json lint-staged entry
for php-cs-fixer may now be dead config because .husky/pre-commit runs
php-cs-fixer directly via RUN/xargs instead of npx lint-staged. Verify whether
any other hook or workflow still uses that lint-staged task; if not, remove or
consolidate the redundant package.json setting so there is a single source of
truth for the php-cs-fixer invocation.

14-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

pre-push と重複する実行環境判定ロジック

Line 14-24 の Docker/host 判定ロジックは .husky/pre-push の Line 11-21 とほぼ同一です。共通スクリプト(例: .husky/lib/detect-runner.sh)に切り出して両フックから source すると、今後の変更漏れやドリフトを防げます。

🤖 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 @.husky/pre-commit around lines 14 - 24, The Docker/host execution
environment detection in the pre-commit hook is duplicated with the pre-push
hook, so extract the shared runner selection logic into a common helper script
and have both hooks source it. Move the current case-based detection from the
pre-commit hook into a reusable script (for example, a shared Husky library),
then update the pre-commit and pre-push hooks to call that shared logic instead
of maintaining separate copies.
.husky/pre-push (2)

11-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

実行環境判定ロジックの重複

PR概要によると pre-commit にも同様の docker/host 判定ロジックが実装される想定です。この判定処理(DC/SVC定義、auto判定、RUN組み立て)を共通シェルスクリプト(例: .husky/_/runner.sh)に切り出し、両フックから source する形にすると保守性が向上します。

🤖 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 @.husky/pre-push around lines 11 - 21, The docker/host execution environment
detection in the pre-push hook is duplicated and should be shared with
pre-commit. Extract the DC/SVC setup, ECCUBE_HOOK_RUNNER auto/host/docker/skip
branching, and RUN construction into a common shell helper such as a sourced
runner script, then update the pre-push hook and the matching pre-commit hook to
load that shared logic instead of duplicating the case block.

17-17: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Docker疎通確認にタイムアウトが無い

$DC exec -T "$SVC" true がDocker daemon無応答時に無期限にハングする可能性があります。ローカル開発フローが固まるのを防ぐため、timeout コマンドでラップすることを推奨します。

🛠️ 修正案
-    if $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
+    if timeout 5 $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
🤖 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 @.husky/pre-push at line 17, The Docker connectivity check in the pre-push
hook can hang indefinitely when the daemon is unresponsive; update the
conditional check in the pre-push script around the $DC exec -T "$SVC" true
probe to run under a timeout wrapper. Keep the existing RUN assignment flow, but
ensure the Docker sanity check fails fast instead of blocking the local
workflow.
🤖 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.

Inline comments:
In @.husky/pre-commit:
- Around line 26-32: The pre-commit hook in the staged PHP fix flow is re-adding
whole files with git add, which can pull in unstaged edits from the same file.
Update the .husky/pre-commit logic around the php-cs-fixer and git add steps so
it only operates on the indexed content, using a stash/restore approach such as
git stash --keep-index before running the fix and restoring afterward. Keep the
fix localized to the existing STAGED_PHP handling and the php-cs-fixer
invocation so only staged changes are committed.
- Around line 11-24: The pre-commit hook in .husky/pre-commit should reject
unsupported ECCUBE_HOOK_RUNNER values instead of silently falling through to
host execution, so update the case handling around RUN to fail explicitly on
unknown input. Also, wherever STAGED_PHP is piped into xargs for php-cs-fixer
and git add, switch to a NUL-safe file list flow so filenames with spaces are
preserved; adjust the hook’s STAGED_PHP processing and the php-cs-fixer/git add
invocation together.

In @.husky/pre-push:
- Line 18: The host check in the pre-push hook only verifies vendor/bin/phpstan,
so it can pass even when vendor/bin/rector is missing and later fail at
execution time instead of showing the intended skip message. Update the
conditional in .husky/pre-push to also require rector’s executable presence
alongside phpstan, matching the later rector invocation path and keeping the
host detection in sync with the commands that actually run.
- Around line 12-21: The ECCUBE_HOOK_RUNNER switch in .husky/pre-push has no
default branch, so invalid values fall through with RUN unset and the hook
continues ambiguously. Update the case handling around ECCUBE_HOOK_RUNNER to add
an explicit default path that rejects unknown values with a clear error message
and non-zero exit, using the existing RUN selection logic for docker, host,
skip, and auto as the valid symbols to anchor the change.

---

Nitpick comments:
In @.husky/pre-commit:
- Around line 1-32: The package.json lint-staged entry for php-cs-fixer may now
be dead config because .husky/pre-commit runs php-cs-fixer directly via
RUN/xargs instead of npx lint-staged. Verify whether any other hook or workflow
still uses that lint-staged task; if not, remove or consolidate the redundant
package.json setting so there is a single source of truth for the php-cs-fixer
invocation.
- Around line 14-24: The Docker/host execution environment detection in the
pre-commit hook is duplicated with the pre-push hook, so extract the shared
runner selection logic into a common helper script and have both hooks source
it. Move the current case-based detection from the pre-commit hook into a
reusable script (for example, a shared Husky library), then update the
pre-commit and pre-push hooks to call that shared logic instead of maintaining
separate copies.

In @.husky/pre-push:
- Around line 11-21: The docker/host execution environment detection in the
pre-push hook is duplicated and should be shared with pre-commit. Extract the
DC/SVC setup, ECCUBE_HOOK_RUNNER auto/host/docker/skip branching, and RUN
construction into a common shell helper such as a sourced runner script, then
update the pre-push hook and the matching pre-commit hook to load that shared
logic instead of duplicating the case block.
- Line 17: The Docker connectivity check in the pre-push hook can hang
indefinitely when the daemon is unresponsive; update the conditional check in
the pre-push script around the $DC exec -T "$SVC" true probe to run under a
timeout wrapper. Keep the existing RUN assignment flow, but ensure the Docker
sanity check fails fast instead of blocking the local workflow.
🪄 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: 80f9a8ba-6b5d-4cd8-896b-fe0ba7d1e3a5

📥 Commits

Reviewing files that changed from the base of the PR and between a387a99 and 5ec08ed.

📒 Files selected for processing (2)
  • .husky/pre-commit
  • .husky/pre-push

Comment thread .husky/pre-commit
Comment on lines +11 to +24
STAGED_PHP=$(git diff --cached --name-only --diff-filter=ACMR | grep '\.php$' || true)
[ -z "$STAGED_PHP" ] && exit 0

DC="docker compose"; SVC="ec-cube"
case "${ECCUBE_HOOK_RUNNER:-auto}" in
docker) RUN="$DC exec -T $SVC" ;;
host) RUN="" ;;
skip) exit 0 ;;
auto)
if $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/php-cs-fixer ]; then RUN=""
else echo "pre-commit: no container / no host php+vendor; skip (HUSKY=0 to bypass)"; exit 0
fi ;;
esac

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 現状 .php ファイルに空白を含むものが存在するか確認
git ls-files -z | tr '\0' '\n' | grep -P '\.php$' | grep ' ' || echo "現状は空白を含む.phpファイルは無し"

Repository: EC-CUBE/ec-cube

Length of output: 173


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 対象ファイルと周辺行を確認
git ls-files .husky/pre-commit
wc -l .husky/pre-commit
cat -n .husky/pre-commit | sed -n '1,220p'

# 空白を含む .php ファイルの有無を再確認しつつ、関連する引数展開の箇所を検索
rg -n --hidden --glob '.husky/pre-commit' 'xargs|git add|STAGED_PHP|ECCUBE_HOOK_RUNNER|php-cs-fixer|RUN=' .husky/pre-commit

Repository: EC-CUBE/ec-cube

Length of output: 2341


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import os, subprocess, textwrap, shlex, tempfile, json

# 1) xargs tokenization with whitespace in file names
sample = "src/Foo Bar.php\nsrc/Baz.php\n"
p = subprocess.run(
    ["sh", "-lc", "printf '%s' \"$1\" | xargs printf '<%s>\\n'"],
    input="",
    text=True,
    capture_output=True,
)
# Use a shell that takes the sample via env to avoid quoting issues
p = subprocess.run(
    ["sh", "-lc", "printf '%s' \"$STAGED_PHP\" | xargs printf '<%s>\\n'"],
    text=True,
    capture_output=True,
    env={**os.environ, "STAGED_PHP": sample},
)
print("xargs_split_stdout:")
print(p.stdout.strip())

# 2) Simulate the unresolved RUN expansion when ECCUBE_HOOK_RUNNER is invalid
script = r'''
DC="docker compose"; SVC="ec-cube"
RUN=""
STAGED_PHP="src/Foo Bar.php\nsrc/Baz.php"
echo "$STAGED_PHP" | xargs $RUN printf '<%s>\n'
'''
p2 = subprocess.run(["sh", "-lc", script], text=True, capture_output=True)
print("cmd_stdout:")
print(p2.stdout.strip())
print("cmd_stderr:")
print(p2.stderr.strip())
PY

Repository: EC-CUBE/ec-cube

Length of output: 338


.husky/pre-commit:15-32 は未知値をエラーにし、パスを NUL 区切りで渡してください

  • ECCUBE_HOOK_RUNNER の未知値は RUN="" のまま host 実行に落ちるので、*) exit 1 ;; などで明示的に弾いた方が安全です。
  • echo "$STAGED_PHP" | xargs ...xargs git add は空白を含むファイル名を分割します。xargs -0 などに置き換えてください。
🤖 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 @.husky/pre-commit around lines 11 - 24, The pre-commit hook in
.husky/pre-commit should reject unsupported ECCUBE_HOOK_RUNNER values instead of
silently falling through to host execution, so update the case handling around
RUN to fail explicitly on unknown input. Also, wherever STAGED_PHP is piped into
xargs for php-cs-fixer and git add, switch to a NUL-safe file list flow so
filenames with spaces are preserved; adjust the hook’s STAGED_PHP processing and
the php-cs-fixer/git add invocation together.

Comment thread .husky/pre-commit
Comment on lines +26 to +32
echo "pre-commit: php-cs-fixer (staged)"
if ! echo "$STAGED_PHP" | xargs $RUN vendor/bin/php-cs-fixer \
--config=.php-cs-fixer.dist.php --path-mode=intersection fix; then
echo "FAIL: php-cs-fixer"
exit 1
fi
echo "$STAGED_PHP" | xargs git add

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

git add が未ステージの変更まで巻き込む可能性

STAGED_PHP に含まれるファイルは fix 後に無条件で git add されます。ユーザーが git add -p 等で一部のみをステージし、同ファイルにさらに未ステージの編集を残していた場合、その未ステージの変更ごと今回のコミットに巻き込まれてしまいます。これは元の lint-staged が内部で行っていた stash による保護(未ステージ差分の退避・復元)が無いために生じる回帰です。

git stash --keep-index 等でステージ済み差分のみを対象に fix を適用し、後で unstash する仕組みの導入を検討してください。

🤖 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 @.husky/pre-commit around lines 26 - 32, The pre-commit hook in the staged
PHP fix flow is re-adding whole files with git add, which can pull in unstaged
edits from the same file. Update the .husky/pre-commit logic around the
php-cs-fixer and git add steps so it only operates on the indexed content, using
a stash/restore approach such as git stash --keep-index before running the fix
and restoring afterward. Keep the fix localized to the existing STAGED_PHP
handling and the php-cs-fixer invocation so only staged changes are committed.

Comment thread .husky/pre-push
Comment on lines +12 to +21
case "${ECCUBE_HOOK_RUNNER:-auto}" in
docker) RUN="$DC exec -T $SVC" ;;
host) RUN="" ;;
skip) exit 0 ;;
auto)
if $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/phpstan ]; then RUN=""
else echo "pre-push: no container / no host php+vendor; skip (HUSKY=0 to bypass)"; exit 0
fi ;;
esac

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

ECCUBE_HOOK_RUNNER に不正な値を指定した場合のフォールバックが暗黙的

case 文に *) のデフォルト節が無いため、docker/host/skip/auto 以外の値(タイプミス等)が指定されると、どのパターンにもマッチせず RUN が未設定のまま処理が継続します。結果として意図せず host 実行として扱われ、host 環境が無い場合は分かりにくいエラーになります。明示的なエラー処理を追加すべきです。

🛠️ 修正案
   auto)
     if $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
     elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/phpstan ]; then RUN=""
     else echo "pre-push: no container / no host php+vendor; skip (HUSKY=0 to bypass)"; exit 0
     fi ;;
+  *)
+    echo "pre-push: invalid ECCUBE_HOOK_RUNNER='$ECCUBE_HOOK_RUNNER' (docker|host|skip|auto)"
+    exit 1 ;;
 esac
📝 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
case "${ECCUBE_HOOK_RUNNER:-auto}" in
docker) RUN="$DC exec -T $SVC" ;;
host) RUN="" ;;
skip) exit 0 ;;
auto)
if $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/phpstan ]; then RUN=""
else echo "pre-push: no container / no host php+vendor; skip (HUSKY=0 to bypass)"; exit 0
fi ;;
esac
case "${ECCUBE_HOOK_RUNNER:-auto}" in
docker) RUN="$DC exec -T $SVC" ;;
host) RUN="" ;;
skip) exit 0 ;;
auto)
if $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/phpstan ]; then RUN=""
else echo "pre-push: no container / no host php+vendor; skip (HUSKY=0 to bypass)"; exit 0
fi ;;
*)
echo "pre-push: invalid ECCUBE_HOOK_RUNNER='$ECCUBE_HOOK_RUNNER' (docker|host|skip|auto)"
exit 1 ;;
esac
🤖 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 @.husky/pre-push around lines 12 - 21, The ECCUBE_HOOK_RUNNER switch in
.husky/pre-push has no default branch, so invalid values fall through with RUN
unset and the hook continues ambiguously. Update the case handling around
ECCUBE_HOOK_RUNNER to add an explicit default path that rejects unknown values
with a clear error message and non-zero exit, using the existing RUN selection
logic for docker, host, skip, and auto as the valid symbols to anchor the
change.

Comment thread .husky/pre-push
skip) exit 0 ;;
auto)
if $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/phpstan ]; then RUN=""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

host判定時にrectorの存在確認が漏れている

[ -x vendor/bin/phpstan ] のみを確認しており、25行目で先に実行される vendor/bin/rector の存在確認がありません。rectorが無い host 環境では判定は通過するものの、実行時にコマンド不存在エラーとなり、意図した「skipメッセージ」が表示されません。

🛠️ 修正案
-    elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/phpstan ]; then RUN=""
+    elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/rector ] && [ -x vendor/bin/phpstan ]; then RUN=""
📝 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
elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/phpstan ]; then RUN=""
elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/rector ] && [ -x vendor/bin/phpstan ]; then RUN=""
🤖 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 @.husky/pre-push at line 18, The host check in the pre-push hook only
verifies vendor/bin/phpstan, so it can pass even when vendor/bin/rector is
missing and later fail at execution time instead of showing the intended skip
message. Update the conditional in .husky/pre-push to also require rector’s
executable presence alongside phpstan, matching the later rector invocation path
and keeping the host detection in sync with the commands that actually run.

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.39%. Comparing base (a387a99) to head (5ec08ed).

Additional details and impacted files
@@            Coverage Diff             @@
##              4.4    #6900      +/-   ##
==========================================
+ Coverage   75.32%   75.39%   +0.06%     
==========================================
  Files         519      519              
  Lines       25483    25483              
==========================================
+ Hits        19196    19212      +16     
+ Misses       6287     6271      -16     
Flag Coverage Δ
Unit 75.39% <ø> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread .husky/pre-commit
echo "FAIL: php-cs-fixer"
exit 1
fi
echo "$STAGED_PHP" | xargs git add

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

部分ステージした内容以外まで、意図せずコミットに混入します

php-cs-fixer はワーキングツリー上のファイルを整形し、その後 git add <file>ファイル全体を再ステージします。そのため git add -p で部分ステージした場合や、git add 後に追加編集した場合、未ステージの変更まで一緒にコミットされてしまいます。従来の npx lint-staged は未ステージ差分を stash で退避してから整形・add・復元するため、この問題を防いでいました。

Emacs 等で部分コミット(git add -p 相当)を多用する運用では実害が出ます。そこで lint-staged を残し、lint-staged が使えない環境でのみ本フックのロジックにフォールバックする形を提案します(node 無しコンテナ対応という本来の目的は保ちつつ、部分ステージ安全性を取り戻せます)。

# 例: lint-staged が使えるならそれを使い、無ければフォールバック
if [ -z "$RUN" ] && [ -x node_modules/.bin/lint-staged ]; then
  npx lint-staged -q && exit 0
fi
# --- 以降が本PRの php-cs-fixer + git add フォールバック ---
# (このフォールバック経路は部分ステージ非対応である旨を出力しておくと親切です)

コンテナ内に node があるか、host 実行時に lint-staged が入っているケースをカバーできれば、多くの環境で従来の安全性を維持できます。

Comment thread .husky/pre-push
[ -n "$RUN" ] && echo "pre-push: running in Docker ($SVC)" || echo "pre-push: running on host"

echo "[1/2] rector (dry-run)"
if ! $RUN vendor/bin/rector process --dry-run --ansi --config=rector.php; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rector が dev debug container を要求するため、cache 未生成の環境で push がブロックされます(必須対応)

rector.php->withSymfonyContainerXml(__DIR__.'/var/cache/dev/Eccube_KernelDevDebugContainer.xml') を指定しており、この XML が存在しないと rector は全ファイルで read error になり exit 非ゼロ → pre-push が失敗します。bin/console cache:clear 直後・fresh clone・CI 相当の cold な状態で git push すると、rector エラーで push できなくなります。手元検証が通ったのは、コンテナの dev キャッシュが既に温まっていたためと思われます。

rector 実行前に dev キャッシュを warmup しておく必要があります:

$RUN bin/console cache:warmup --env=dev

(毎回 warmup が重いようなら、XML の存在チェック → 無ければ warmup、という分岐でも可)

あわせて軽微な点として、auto 判定(L18)は [ -x vendor/bin/phpstan ] を見ていますが、このフックで最初に走るのは vendor/bin/rector です。整合のため rector の存在も確認しておくとより安全です。

@nanasess nanasess left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

コメント確認お願い致します

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants