fix: eliminate code duplication with shared modules and composite actions - #652
Conversation
…ions Extract duplicated patterns across hooks, shell scripts, and workflows: - Create hooks/common.py shared module (JSON parsing, output formatting, git ops) - Consolidate lib/config.sh 8 copy-paste functions into 2 generic functions - Replace color/log definitions in 10 shell scripts with lib/output.sh sourcing - Create .github/actions/setup-node-ci composite action for 3 workflows - Standardize test assertion style (it -> test) in credential-filtering tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRefactors multiple .claude hook scripts to use a new shared Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🔍 AI Code Review (Local Hook)Models: Codex (default) / Gemini (default) 🤖 Codex Review指摘事項 判定 🤖 Generated by post_pr_ai_review.py hook |
![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: 0732b232a4
ℹ️ 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".
| log_error() { echo -e "${RED}[ERROR]${NC} $1"; } | ||
| # Source shared output library (provides log_info, log_success, log_warn, color vars) | ||
| # shellcheck source=/dev/null | ||
| source "$SCRIPT_DIR/lib/output.sh" 2>/dev/null || { |
There was a problem hiding this comment.
This refactor makes source "$SCRIPT_DIR/lib/output.sh" the default path, but output.sh does not provide log_error, and log_error is now only defined inside the fallback block. When any error path is taken (for example invalid args at --bogus or a failed sub-step), the script crashes with log_error: command not found instead of reporting the intended error and continuing its failure handling.
Useful? React with 👍 / 👎.
| log_error() { echo -e "${RED}[ERROR]${NC} $1"; } | ||
| # Source shared output library (provides log_info, log_success, log_warn, color vars) | ||
| # shellcheck source=/dev/null | ||
| source "$SCRIPT_DIR/lib/output.sh" 2>/dev/null || { |
There was a problem hiding this comment.
In the normal execution path where lib/output.sh is successfully sourced, log_error is undefined (that helper file exports log_info/log_success/log_warn only), so the required-command check now fails with command not found when gh is missing. This breaks the script's explicit dependency error handling and produces an opaque failure instead of the intended actionable message.
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: 5
🧹 Nitpick comments (5)
.github/workflows/ci.yml (1)
164-164: Consider whether npm dependencies are needed for integration tests.The integration-test job runs
.batsfiles that validate npm command availability and check for npm usage in workflow files (via grep patterns), but don't execute npm packages directly. Ifinstall: 'false'is passed, Node.js setup still provides thenpmcommand, allowing tests likecommand_exists npmto pass while skipping the slowernpm cistep.💡 Potential optimization if npm package installation isn't needed
- uses: ./.github/actions/setup-node-ci + with: + install: 'false'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 164, The integration-test job currently invokes the custom action setup-node-ci which installs npm packages; if the .bats integration tests only need the npm binary (not installed packages), update the job to skip npm package installation by providing the action input to disable install (e.g., pass install: 'false' to the setup-node-ci action) or remove the action entirely and only ensure Node/npm is available; locate references to the integration-test job and the uses: ./.github/actions/setup-node-ci entry and change the action invocation to pass the install flag so npm is available but npm ci is skipped..claude/hooks/common.py (2)
47-49: Minor:-hsubstring match may have false positives.The current check
"-h" in commandcould match substrings in paths or other flags (e.g.,--http,/path-here/). Since this is used for early-exit optimization, false positives are safe but may skip processing unnecessarily.♻️ Optional: Use word boundary matching
+import re + def is_help_command(command: str) -> bool: """Check if command is a help/dry-run command.""" - return "--help" in command or "-h" in command + return bool(re.search(r'--help\b', command) or re.search(r'\s-h\b', command))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/common.py around lines 47 - 49, The is_help_command function currently uses substring checks which cause false positives (e.g., matching "-h" inside paths or other flags); change it to detect standalone help flags by parsing the command into arguments or using word-boundary regex checks so only an exact "-h" or "--help" token triggers true (update is_help_command to split the command into args or apply regex like token boundaries and return True only when "-h" or "--help" appears as a separate token).
1-16: Add unit tests for this shared module.This module provides critical utilities (input parsing, command extraction, Git operations) used by 11 hook files. The existing test suite only validates syntax and basic structure; specific tests for these utility functions would prevent regressions across dependent hooks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/common.py around lines 1 - 16, Add focused unit tests for the shared utilities in .claude/hooks/common.py: write tests that (1) validate JSON input parsing from stdin (simulate sys.stdin to exercise the parse function that reads JSON), (2) exercise tool context extraction (call the function that extracts tool/context fields from input objects and assert expected outputs and error handling), (3) verify output formatting helpers (headers, sections, status functions produce expected strings), (4) stub subprocess/git interactions to test Git operation helpers (e.g., the function that runs git commands returns expected output and handles failures), and (5) test package manager detection (feed different filesystem/project layouts or mocked Path.exists to assert detection logic). Use pytest with fixtures and monkeypatch to simulate stdin, subprocess, and filesystem, and include edge cases and error paths for each referenced function to prevent regressions across the 11 dependent hooks.script/test-coverage-trend.sh (1)
9-12: Consider removing the SC2034 disable for consistency.This file includes
# shellcheck disable=SC2034(line 10) inside the fallback block, but the other 7 scripts using this same pattern do not. All color variables (RED,GREEN,YELLOW,BLUE,NC) are actually used in this script, so the suppression appears unnecessary.For consistency across the refactored scripts, consider removing the extra directive:
♻️ Suggested diff
# shellcheck source=/dev/null source "$SCRIPT_DIR/lib/output.sh" 2>/dev/null || { - # shellcheck disable=SC2034 readonly RED='\033[0;31m' GREEN='\033[0;32m' YELLOW='\033[1;33m' BLUE='\033[0;34m' NC='\033[0m' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/test-coverage-trend.sh` around lines 9 - 12, Remove the unnecessary shellcheck suppression: delete the comment "# shellcheck disable=SC2034" from the fallback block so the linter runs normally, leaving the readonly color variable declarations (RED, GREEN, YELLOW, BLUE, NC) intact; this keeps the readonly assignments in the fallback while matching the pattern used in the other scripts.script/lib/config.sh (1)
211-223: Clarify Gemini's "warning only" intent with an explicit directory check.The
|| return 0pattern catches both missing source directory and other failures from the helper function. While the helper already handles missing directories gracefully, the wrapper's broad exception clause masks other potential errors. An explicit pre-check for the directory keeps the "warning only" case visible and lets actual helper failures propagate normally.Note:
script/lib/config.shis listed inscript/.shellcheck-exclude, so no static validation is enforced on this file.💡 Suggested refactor
config::import_gemini() { + local source_dir="${1:?Source directory required}" + local target_dir="${2:-$HOME/.gemini}" + + if [[ ! -d "$source_dir" ]]; then + echo "⚠️ Gemini設定ディレクトリが見つかりません: $source_dir" + return 0 + fi + config::_import_tool "Gemini" \ - "${1:?Source directory required}" \ - "${2:-$HOME/.gemini}" \ - CONFIG_GEMINI_SHARED_FILES || return 0 # Gemini: warning only + "$source_dir" \ + "$target_dir" \ + CONFIG_GEMINI_SHARED_FILES } config::export_gemini() { + local source_dir="${1:-$HOME/.gemini}" + local target_dir="${2:?Target directory required}" + + if [[ ! -d "$source_dir" ]]; then + echo "⚠️ Gemini設定ディレクトリが見つかりません: $source_dir" + return 0 + fi + config::_export_tool "Gemini" \ - "${1:-$HOME/.gemini}" \ - "${2:?Target directory required}" \ - CONFIG_GEMINI_SHARED_FILES || return 0 # Gemini: warning only + "$source_dir" \ + "$target_dir" \ + CONFIG_GEMINI_SHARED_FILES }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/lib/config.sh` around lines 211 - 223, The wrapper functions config::import_gemini and config::export_gemini should perform an explicit directory existence check for the source/target path before calling config::_import_tool / config::_export_tool: test the respective directory argument (the first arg for import, the second arg for export, using the same default expansion as currently present) and if it does not exist return 0 (preserving the "warning only" behavior); remove the trailing "|| return 0" so that any real failure from config::_import_tool or config::_export_tool (referencing CONFIG_GEMINI_SHARED_FILES) will propagate normally.
🤖 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/hooks/block_config_edit.py:
- Around line 11-15: Remove the unused import of the json module: delete the
line importing json at the top of .claude/hooks/block_config_edit.py since the
code now uses load_hook_input() (function referenced as load_hook_input) and no
other code uses json; keep the import of load_hook_input and the existing data =
load_hook_input() line intact.
In @.claude/hooks/block_dangerous_commands.py:
- Around line 8-14: Remove the unused json import from the top-level imports;
edit the import block so it only imports sys, re, and the common helpers (remove
the line "import json"), leaving load_hook_input and get_command usage unchanged
(symbols to check: load_hook_input, get_command, data, cmd).
In @.claude/hooks/block_git_no_verify.py:
- Around line 2-8: Remove the unused json import from the top of the file;
locate the import line that currently reads "import json" alongside "import sys"
and "import shlex" and delete it so only required modules remain, since
load_hook_input() and get_command() are used and json is no longer referenced.
In `@script/lib/config.sh`:
- Around line 78-79: Replace the fragile import copy and suppressed errors:
instead of cp -r "$source_dir/$dir"/* "$target_dir/$dir/" 2>/dev/null || true,
use cp -R "$source_dir/$dir"/. "$target_dir/$dir/" and allow failures to
propagate (or guard with && for the success echo), e.g. cp -R
"$source_dir/$dir"/. "$target_dir/$dir/" && echo "✅ Imported $display_name";
likewise update the corresponding export copy that used the wildcard pattern to
use cp -R "$export_source"/. "$export_target"/ (remove the 2>/dev/null || true
suppression) so hidden files are copied and real errors are reported; reference
the variables source_dir, dir, target_dir, display_name (and the export
source/target vars) when making the changes.
In `@script/update-all.sh`:
- Around line 11-19: The fallback aliases in the shared output library are
missing the log_error alias, so add a log_error wrapper alongside
log_info/log_success/log_warn that forwards all arguments to output::error
(i.e., implement a log_error function that calls output::error with the received
args) in the same block where log_info/log_success/log_warn are defined so calls
to log_error in update-all.sh succeed.
---
Nitpick comments:
In @.claude/hooks/common.py:
- Around line 47-49: The is_help_command function currently uses substring
checks which cause false positives (e.g., matching "-h" inside paths or other
flags); change it to detect standalone help flags by parsing the command into
arguments or using word-boundary regex checks so only an exact "-h" or "--help"
token triggers true (update is_help_command to split the command into args or
apply regex like token boundaries and return True only when "-h" or "--help"
appears as a separate token).
- Around line 1-16: Add focused unit tests for the shared utilities in
.claude/hooks/common.py: write tests that (1) validate JSON input parsing from
stdin (simulate sys.stdin to exercise the parse function that reads JSON), (2)
exercise tool context extraction (call the function that extracts tool/context
fields from input objects and assert expected outputs and error handling), (3)
verify output formatting helpers (headers, sections, status functions produce
expected strings), (4) stub subprocess/git interactions to test Git operation
helpers (e.g., the function that runs git commands returns expected output and
handles failures), and (5) test package manager detection (feed different
filesystem/project layouts or mocked Path.exists to assert detection logic). Use
pytest with fixtures and monkeypatch to simulate stdin, subprocess, and
filesystem, and include edge cases and error paths for each referenced function
to prevent regressions across the 11 dependent hooks.
In @.github/workflows/ci.yml:
- Line 164: The integration-test job currently invokes the custom action
setup-node-ci which installs npm packages; if the .bats integration tests only
need the npm binary (not installed packages), update the job to skip npm package
installation by providing the action input to disable install (e.g., pass
install: 'false' to the setup-node-ci action) or remove the action entirely and
only ensure Node/npm is available; locate references to the integration-test job
and the uses: ./.github/actions/setup-node-ci entry and change the action
invocation to pass the install flag so npm is available but npm ci is skipped.
In `@script/lib/config.sh`:
- Around line 211-223: The wrapper functions config::import_gemini and
config::export_gemini should perform an explicit directory existence check for
the source/target path before calling config::_import_tool /
config::_export_tool: test the respective directory argument (the first arg for
import, the second arg for export, using the same default expansion as currently
present) and if it does not exist return 0 (preserving the "warning only"
behavior); remove the trailing "|| return 0" so that any real failure from
config::_import_tool or config::_export_tool (referencing
CONFIG_GEMINI_SHARED_FILES) will propagate normally.
In `@script/test-coverage-trend.sh`:
- Around line 9-12: Remove the unnecessary shellcheck suppression: delete the
comment "# shellcheck disable=SC2034" from the fallback block so the linter runs
normally, leaving the readonly color variable declarations (RED, GREEN, YELLOW,
BLUE, NC) intact; this keeps the readonly assignments in the fallback while
matching the pattern used in the other scripts.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8fab8024-93f5-4630-9b1f-f83a59acffd9
📒 Files selected for processing (28)
.claude/hooks/block_config_edit.py.claude/hooks/block_dangerous_commands.py.claude/hooks/block_git_no_verify.py.claude/hooks/common.py.claude/hooks/post_commit_adr_reminder.py.claude/hooks/post_edit_auto_lint.py.claude/hooks/post_git_push_ci.py.claude/hooks/post_pr_ai_review.py.claude/hooks/post_pr_ci_watch.py.claude/hooks/pre_exit_plan_ai_review.py.claude/hooks/pre_git_quality_gates.py.claude/hooks/stop_test_verification.py.github/actions/setup-node-ci/action.yml.github/workflows/ci.yml.github/workflows/security.yml.github/workflows/update-libraries.ymlscript/branch-cleanup.shscript/changelog-generator.shscript/code-complexity-check.shscript/container-health.shscript/dependency-health-check.shscript/lib/config.shscript/pre-pr-checklist.shscript/security-credential-scan.shscript/test-coverage-trend.shscript/update-actions.shscript/update-all.shtest/credential-filtering.test.js
| import sys | ||
| import json | ||
| from common import load_hook_input | ||
|
|
||
| data = json.load(sys.stdin) | ||
| data = load_hook_input() |
There was a problem hiding this comment.
Remove unused json import.
The json module is imported but no longer used after refactoring to use load_hook_input().
🧹 Proposed fix
import sys
-import json
from common import load_hook_input🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/hooks/block_config_edit.py around lines 11 - 15, Remove the unused
import of the json module: delete the line importing json at the top of
.claude/hooks/block_config_edit.py since the code now uses load_hook_input()
(function referenced as load_hook_input) and no other code uses json; keep the
import of load_hook_input and the existing data = load_hook_input() line intact.
| import sys | ||
| import json | ||
| import re | ||
| from common import load_hook_input, get_command | ||
|
|
||
| data = json.load(sys.stdin) | ||
| cmd = (data.get("tool_input", {}) or {}).get("command") or "" | ||
| data = load_hook_input() | ||
| cmd = get_command(data) |
There was a problem hiding this comment.
Remove unused json import.
The json module is imported but no longer used after refactoring to use load_hook_input() from common.
🧹 Proposed fix
import sys
-import json
import re
from common import load_hook_input, get_command📝 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.
| import sys | |
| import json | |
| import re | |
| from common import load_hook_input, get_command | |
| data = json.load(sys.stdin) | |
| cmd = (data.get("tool_input", {}) or {}).get("command") or "" | |
| data = load_hook_input() | |
| cmd = get_command(data) | |
| import sys | |
| import re | |
| from common import load_hook_input, get_command | |
| data = load_hook_input() | |
| cmd = get_command(data) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/hooks/block_dangerous_commands.py around lines 8 - 14, Remove the
unused json import from the top-level imports; edit the import block so it only
imports sys, re, and the common helpers (remove the line "import json"), leaving
load_hook_input and get_command usage unchanged (symbols to check:
load_hook_input, get_command, data, cmd).
| import sys | ||
| import json | ||
| import shlex | ||
| from common import load_hook_input, get_command | ||
|
|
||
| # Read input from Claude | ||
| data = json.load(sys.stdin) | ||
| cmd = (data.get("tool_input", {}) or {}).get("command") or "" | ||
| data = load_hook_input() | ||
| cmd = get_command(data) |
There was a problem hiding this comment.
Remove unused json import.
The json module is imported but no longer used after refactoring to use load_hook_input() and get_command().
🧹 Proposed fix
import sys
-import json
import shlex
from common import load_hook_input, get_command📝 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.
| import sys | |
| import json | |
| import shlex | |
| from common import load_hook_input, get_command | |
| # Read input from Claude | |
| data = json.load(sys.stdin) | |
| cmd = (data.get("tool_input", {}) or {}).get("command") or "" | |
| data = load_hook_input() | |
| cmd = get_command(data) | |
| import sys | |
| import shlex | |
| from common import load_hook_input, get_command | |
| data = load_hook_input() | |
| cmd = get_command(data) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/hooks/block_git_no_verify.py around lines 2 - 8, Remove the unused
json import from the top of the file; locate the import line that currently
reads "import json" alongside "import sys" and "import shlex" and delete it so
only required modules remain, since load_hook_input() and get_command() are used
and json is no longer referenced.
| cp -r "$source_dir/$dir"/* "$target_dir/$dir/" 2>/dev/null || true | ||
| echo "✅ Imported $display_name" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
command -v zsh >/dev/null || { echo "zsh not installed"; exit 1; }
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
export CODERABBIT_TMPDIR="$tmpdir"
zsh <<'ZSH'
set -euo pipefail
tmpdir="$CODERABBIT_TMPDIR"
mkdir -p "$tmpdir/src/dir" "$tmpdir/dst/dir"
touch "$tmpdir/src/dir/visible" "$tmpdir/src/dir/.hidden"
cp -r "$tmpdir/src/dir"/* "$tmpdir/dst/dir/" 2>/dev/null || true
print '--- destination after current pattern ---'
ls -la "$tmpdir/dst/dir"
mkdir -p "$tmpdir/src2/dir" "$tmpdir/dst2/dir"
touch "$tmpdir/src2/dir/visible"
chmod 500 "$tmpdir/dst2/dir"
if (cp -r "$tmpdir/src2/dir"/* "$tmpdir/dst2/dir/" 2>/dev/null || true); then
print "masked_failure=true"
fi
ZSHRepository: keito4/config
Length of output: 74
🏁 Script executed:
# Check the file and context around lines 78-79
head -n 85 script/lib/config.sh | tail -n +70Repository: keito4/config
Length of output: 550
🏁 Script executed:
# Check if there's a shellcheck exclude file
cat script/.shellcheck-exclude 2>/dev/null || echo "No exclude file found"Repository: keito4/config
Length of output: 213
🏁 Script executed:
# Check if shellcheck is available and can validate the script
command -v shellcheck >/dev/null && echo "shellcheck found" || echo "shellcheck not found"Repository: keito4/config
Length of output: 73
🏁 Script executed:
# Verify glob behavior with bash (since zsh not available)
# Test if * glob skips dotfiles
bash -c '
mkdir -p /tmp/test_src /tmp/test_dst
touch /tmp/test_src/visible /tmp/test_src/.hidden
echo "Source contents:"
ls -la /tmp/test_src/
echo ""
echo "Testing glob expansion:"
cd /tmp/test_src
for file in *; do
echo " Matched: $file"
done
echo ""
echo "After cp -r /tmp/test_src/* /tmp/test_dst/:"
cp -r /tmp/test_src/* /tmp/test_dst/ 2>/dev/null || true
ls -la /tmp/test_dst/
rm -rf /tmp/test_src /tmp/test_dst
'Repository: keito4/config
Length of output: 548
🏁 Script executed:
# Get broader context around the import function
sed -n '60,85p' script/lib/config.shRepository: keito4/config
Length of output: 820
🏁 Script executed:
# Check the export function at lines 137-139
sed -n '130,145p' script/lib/config.shRepository: keito4/config
Length of output: 639
🏁 Script executed:
# Check what CONFIG_GEMINI_SHARED_DIRS contains
grep -n "CONFIG_GEMINI_SHARED_DIRS\|SHARED_DIRS" script/lib/config.sh | head -20Repository: keito4/config
Length of output: 475
🏁 Script executed:
# Check if these directories exist and if they contain any dotfiles
for dir in commands agents hooks prompts rules; do
if fd -H "^\." --base-directory "$dir" 2>/dev/null | head -5; then
echo "Found dotfiles in $dir"
fi
doneRepository: keito4/config
Length of output: 182
🏁 Script executed:
# Also check in common tool config directories for these patterns
fd -H "^\." --base-directory . --type f -x grep -l "commands\|agents\|hooks\|prompts\|rules" {} \; 2>/dev/null | head -10Repository: keito4/config
Length of output: 102
🏁 Script executed:
# Show what dotfiles exist in each of these directories
for dir in commands agents hooks prompts rules; do
echo "=== Dotfiles in $dir ==="
fd -H "^\." --base-directory "$dir" --type f 2>/dev/null | head -10
doneRepository: keito4/config
Length of output: 192
🏁 Script executed:
# Test the proposed fix to ensure it works correctly
bash -c '
mkdir -p /tmp/fix_test_src /tmp/fix_test_dst
touch /tmp/fix_test_src/visible /tmp/fix_test_src/.hidden /tmp/fix_test_src/.gitkeep
echo "Source:"
ls -la /tmp/fix_test_src/
echo ""
echo "Testing proposed fix: cp -R /tmp/fix_test_src/. /tmp/fix_test_dst/"
cp -R /tmp/fix_test_src/. /tmp/fix_test_dst/
echo "Destination:"
ls -la /tmp/fix_test_dst/
rm -rf /tmp/fix_test_src /tmp/fix_test_dst
'Repository: keito4/config
Length of output: 701
Use cp -R with dot-slash syntax to copy directory contents including hidden files and report copy failures.
The pattern "$source_dir/$dir"/* skips dotfiles, and error handling with || true / && ... || ... masks real failures. Use cp -R "$source_dir/$dir"/. "$target_dir/$dir/" instead to copy all files (including hidden ones) and let failures propagate.
Suggested fix for import (lines 78-79)
mkdir -p "$target_dir/$dir"
- cp -r "$source_dir/$dir"/* "$target_dir/$dir/" 2>/dev/null || true
+ cp -R "$source_dir/$dir"/. "$target_dir/$dir/"
echo "✅ Imported $display_name"Suggested fix for export (lines 137-139)
mkdir -p "$target_dir/$dir"
- cp -r "$source_dir/$dir"/* "$target_dir/$dir/" 2>/dev/null && \
- echo "✅ Exported ${label:+$label/}$dir/" || \
- echo "⚠️ No ${label:+$label/}$dir found"
+ cp -R "$source_dir/$dir"/. "$target_dir/$dir/"
+ echo "✅ Exported ${label:+$label/}$dir/"📝 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.
| cp -r "$source_dir/$dir"/* "$target_dir/$dir/" 2>/dev/null || true | |
| echo "✅ Imported $display_name" | |
| cp -R "$source_dir/$dir"/. "$target_dir/$dir/" | |
| echo "✅ Imported $display_name" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@script/lib/config.sh` around lines 78 - 79, Replace the fragile import copy
and suppressed errors: instead of cp -r "$source_dir/$dir"/* "$target_dir/$dir/"
2>/dev/null || true, use cp -R "$source_dir/$dir"/. "$target_dir/$dir/" and
allow failures to propagate (or guard with && for the success echo), e.g. cp -R
"$source_dir/$dir"/. "$target_dir/$dir/" && echo "✅ Imported $display_name";
likewise update the corresponding export copy that used the wildcard pattern to
use cp -R "$export_source"/. "$export_target"/ (remove the 2>/dev/null || true
suppression) so hidden files are copied and real errors are reported; reference
the variables source_dir, dir, target_dir, display_name (and the export
source/target vars) when making the changes.
| # Source shared output library (provides log_info, log_success, log_warn, color vars) | ||
| # shellcheck source=/dev/null | ||
| source "$SCRIPT_DIR/lib/output.sh" 2>/dev/null || { | ||
| GREEN='\033[0;32m' YELLOW='\033[1;33m' BLUE='\033[0;34m' RED='\033[0;31m' NC='\033[0m' | ||
| log_info() { echo -e "${BLUE}[INFO]${NC} $1"; } | ||
| log_success() { echo -e "${GREEN}[SUCCESS]${NC} $1"; } | ||
| log_warn() { echo -e "${YELLOW}[WARN]${NC} $1"; } | ||
| log_error() { echo -e "${RED}[ERROR]${NC} $1"; } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the shared output API exports log_error alias
rg -n '^\s*(output::log_error|log_error)\s*\(' script/lib/output.sh
# Verify affected call sites that require log_error
rg -n '\blog_error\s*\(' script/update-all.sh script/update-actions.shRepository: keito4/config
Length of output: 39
🏁 Script executed:
# Check if files exist and their contents
echo "=== Checking script/lib/output.sh ==="
if [ -f script/lib/output.sh ]; then
echo "File exists. Contents:"
cat -n script/lib/output.sh
else
echo "File does not exist"
fi
echo ""
echo "=== Checking script/update-all.sh for log_error calls ==="
if [ -f script/update-all.sh ]; then
echo "File exists. Searching for log_error:"
grep -n "log_error" script/update-all.sh || echo "No matches found"
else
echo "File does not exist"
fi
echo ""
echo "=== Checking script/update-actions.sh ==="
if [ -f script/update-actions.sh ]; then
echo "File exists. Searching for log_error:"
grep -n "log_error" script/update-actions.sh || echo "No matches found"
else
echo "File does not exist"
fiRepository: keito4/config
Length of output: 6833
log_error is missing from the backward compatibility aliases in script/lib/output.sh
The file defines log_info, log_success, and log_warn aliases (lines 138-140) but omits log_error. When script/update-all.sh sources the library successfully, it skips the fallback block (lines 11-19) that defines log_error, causing calls to log_error at lines 42, 68, 87, 106, and 126 to fail with "command not found".
Add the missing alias to script/lib/output.sh:
log_error() { output::error "$@"; }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@script/update-all.sh` around lines 11 - 19, The fallback aliases in the
shared output library are missing the log_error alias, so add a log_error
wrapper alongside log_info/log_success/log_warn that forwards all arguments to
output::error (i.e., implement a log_error function that calls output::error
with the received args) in the same block where log_info/log_success/log_warn
are defined so calls to log_error in update-all.sh succeed.
Integration tests grepped for "npm ci" and "actions/setup-node@" directly in ci.yml, which now delegates to .github/actions/setup-node-ci composite action. Update patterns to match either direct or composite action usage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.107.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
lib/output.shの source 化で排除setup-node-cicomposite action を作成し、ci.yml / security.yml / update-libraries.yml の Node.js セットアップを一元化it→test統一28ファイル変更、純削減 -54行（457追加 / 511削除）
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
New Features
Tests