fix: リポジトリ品質改善（CI/セキュリティ/ドキュメント/不要ファイル削除） - #483
Conversation
- docker-image.yml: build-args から CLAUDE_CODE_OAUTH_TOKEN を削除（secrets mount のみ使用） - ci.yml: bats テストの || true を削除しテスト失敗を検出可能に - ci.yml: payload-file-path を payload-template-file-path に修正（変数展開対応） - update-libraries.yml: create-pull-request@v6 を @v7 に更新 - package.json: shellcheck 除外を /lib/ 全体から zsh スクリプト4件のみに変更 - update-actions.sh: sed -i を macOS/Linux 両対応に修正 - docs/setup/README.md: checkout@v4 を @v6 に更新、payload-file-path を修正 - .trivyignore: 全 CVE にレビュー日を追加 - devcontainer.json: Node.js/pnpm 重複に TODO コメント追加 - 不要ファイル削除: check-docs-sync.sh, docs-common.js（未使用） Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughRemoved docs-generation tooling and helpers; updated CI workflows (test failure behavior, action versions, Slack input), removed a Docker build-arg, adjusted lint/script tooling, updated docs and metadata comments, and removed related tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Bats as Bats (integration-tests)
participant Docker as Docker Build
participant Registry as Image Registry
participant Slack as Slack Notification
GH->>Bats: run integration-tests
alt tests pass
GH->>Docker: build & push image (omit CLAUDE_CODE_OAUTH_TOKEN build-arg)
Docker->>Registry: push image
else tests fail
Bats-->>GH: exit non-zero (step fails)
GH->>Slack: send failure using payload-template-file-path
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 AI Code Review (Local Hook)Models: Codex (default) / Gemini (default) 🤖 Codex Review指摘事項なし（レビュー対象の差分で新規に導入されたアクション可能な問題は見つかりませんでした）。 patch is correct — 変更による不具合やリスクの増加を確認できませんでした。信頼度 0.62 🤖 Generated by post_pr_ai_review.py hook |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.devcontainer/devcontainer.json (1)
10-10: Consider tracking this TODO in an issue.The TODO comment usefully documents the Node.js duplication concern. To ensure it's addressed and not forgotten, consider creating a tracking issue for removing this redundancy.
Do you want me to help draft the issue description, or would you prefer to track this cleanup separately?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/devcontainer.json at line 10, The TODO in .devcontainer/devcontainer.json notes duplicated Node.js installation (Dockerfile installs v22.14.0 while the feature is kept for pnpm); create a tracking issue in your repo (e.g., "Remove duplicated Node.js install from devcontainer") that describes the duplication, expected resolution (remove Node install from devcontainer or consolidate into Dockerfile), steps to verify (build devcontainer and ensure pnpm still installs), and link this TODO comment to the new issue number or URL so it isn't forgotten; update the TODO to reference the created issue ID/URL..trivyignore (1)
15-16: Verify if Vercel's esbuild 0.27.0 resolves these Go stdlib CVEs.These 2023 Go stdlib
html/templateCVEs (CVE-2023-24538, CVE-2023-24540) have been suppressed for nearly 3 years. Vercel CLI now bundles esbuild 0.27.0 (as of recent releases like v50.1.5). Check the esbuild release notes to confirm whether 0.27.0 was compiled with Go 1.19.9+ or 1.20.4+, which would resolve both CVEs. If so, these.trivyignoreentries can be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.trivyignore around lines 15 - 16, Check whether the suppressed CVE entries (CVE-2023-24538 and CVE-2023-24540) in .trivyignore can be removed by verifying that Vercel's bundled esbuild v0.27.0 was compiled with a Go toolchain new enough to contain the fixes (Go 1.19.9+ or Go 1.20.4+): look up esbuild v0.27.0 release notes and build metadata (or inspect the Vercel CLI release that bundles esbuild v0.27.0) to confirm the Go version used, and if confirmed, remove the CVE lines from .trivyignore and re-run a Trivy scan to ensure the vulnerabilities are no longer reported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/devcontainer.json:
- Around line 11-13: The devcontainer currently sets "pnpmVersion": "latest"
which harms reproducible builds; change the pnpmVersion value to a specific
pinned version (e.g., "7.29.0") and also pin the global installer in the
Dockerfile by replacing the unpinned npm install -g pnpm command with an
installation of that same specific version (ensure both the "pnpmVersion" entry
and the npm install -g pnpm invocation use the identical pinned version string
so builds are deterministic).
---
Nitpick comments:
In @.devcontainer/devcontainer.json:
- Line 10: The TODO in .devcontainer/devcontainer.json notes duplicated Node.js
installation (Dockerfile installs v22.14.0 while the feature is kept for pnpm);
create a tracking issue in your repo (e.g., "Remove duplicated Node.js install
from devcontainer") that describes the duplication, expected resolution (remove
Node install from devcontainer or consolidate into Dockerfile), steps to verify
(build devcontainer and ensure pnpm still installs), and link this TODO comment
to the new issue number or URL so it isn't forgotten; update the TODO to
reference the created issue ID/URL.
In @.trivyignore:
- Around line 15-16: Check whether the suppressed CVE entries (CVE-2023-24538
and CVE-2023-24540) in .trivyignore can be removed by verifying that Vercel's
bundled esbuild v0.27.0 was compiled with a Go toolchain new enough to contain
the fixes (Go 1.19.9+ or Go 1.20.4+): look up esbuild v0.27.0 release notes and
build metadata (or inspect the Vercel CLI release that bundles esbuild v0.27.0)
to confirm the Go version used, and if confirmed, remove the CVE lines from
.trivyignore and re-run a Trivy scan to ensure the vulnerabilities are no longer
reported.
| "ghcr.io/devcontainers/features/node:1": { | ||
| "pnpmVersion": "latest" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see what files exist in the .devcontainer directory
find .devcontainer -type f | head -20Repository: keito4/config
Length of output: 397
🏁 Script executed:
# Read the devcontainer.json file to see the full context
cat -n .devcontainer/devcontainer.jsonRepository: keito4/config
Length of output: 2814
🏁 Script executed:
# Check if there's a Dockerfile in the .devcontainer directory
ls -la .devcontainer/ | grep -i dockerfileRepository: keito4/config
Length of output: 118
🏁 Script executed:
# Read the Dockerfile content
cat -n .devcontainer/DockerfileRepository: keito4/config
Length of output: 9507
Pin pnpm version for reproducible builds.
Setting pnpmVersion: "latest" reduces build reproducibility. However, note that pnpm version non-determinism also exists in the Dockerfile (line 53: npm install -g pnpm without version pinning). Consider pinning a specific pnpm version in both locations.
The TODO comment correctly documents that this feature is kept only for pnpm installation; Node.js version conflict is not a concern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/devcontainer.json around lines 11 - 13, The devcontainer
currently sets "pnpmVersion": "latest" which harms reproducible builds; change
the pnpmVersion value to a specific pinned version (e.g., "7.29.0") and also pin
the global installer in the Dockerfile by replacing the unpinned npm install -g
pnpm command with an installation of that same specific version (ensure both the
"pnpmVersion" entry and the npm install -g pnpm invocation use the identical
pinned version string so builds are deterministic).
![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: cedcafbdaf
ℹ️ 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".
| with: | ||
| channel-id: ${{ vars.SLACK_CI_CHANNEL_ID }} | ||
| payload-file-path: '.github/slack-ci-failure.json' | ||
| payload-template-file-path: '.github/slack-ci-failure.json' |
There was a problem hiding this comment.
In notify-failure, slackapi/slack-github-action@v2.1.1 is configured with payload-template-file-path, but this action expects payload-file-path (and payload-templated: true when template expansion is needed). With the current key, failures on main can skip or break Slack notifications because the payload file is not actually consumed.
Useful? React with 👍 / 👎.
- development-tools.bats: 存在しない setup-new-repo.sh のテストを削除 - container-security.yml: trivy-action@v0.34.0 を @0.34.0 に修正（v prefix なし） Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
(aside) |
|
Claude encountered an error —— View job CI修正作業
|
| # | テスト | 原因 |
|---|---|---|
| 9 | security-credential-scan.sh should scan for credentials |
((CRITICAL_COUNT++)) が set -e 環境でexit codeが1になる（0→1時の後置インクリメント） |
| 35 | output.sh script exists and is executable |
script/lib/output.sh に実行権限がない |
| 65, 66, 72-75 | setup-claude.sh defines color constants/log functions/etc |
setup-claude.sh がリファクタリングされて GREEN=, log_info() 等が lib/ に移動されたが、テストが旧実装を期待している |
| 115 | CI workflow uses secure practices |
テストが checkout@v4 を期待するが、PRで @v6.0.2 に更新 |
| 121 | update-libraries workflow uses pinned third-party actions |
テストが create-pull-request@v6 を期待するが、PRで @v7 に更新 |
| 131 | ci workflow uploads coverage reports securely |
テストが codecov-action@v4 を期待するが、実際は @v5.5.2 |
修正方針：
security-credential-scan.sh:((CRITICAL_COUNT++))→CRITICAL_COUNT=$((CRITICAL_COUNT + 1))script/lib/output.sh:chmod +xで実行権限付与setup_claude.bats: リファクタリング後の実装に合わせてテストを更新workflows.bats: 実際のバージョン文字列に合わせてテストを更新
-
security-credential-scan.shの((CRITICAL_COUNT++))/((WARNING_COUNT++))バグ修正 -
script/lib/output.shに実行権限付与 -
test/integration/setup_claude.batsを現在の実装に合わせて更新 -
test/integration/workflows.batsのバージョン文字列を更新 - 変更をコミット＆プッシュ
|
(aside) |
1 similar comment
|
(aside) |
|
Claude finished @keito4's task in 2m 30s —— View job CI修正完了 ✅
修正内容
コミット
|
ワークフローのアクションバージョンを更新したことに伴い、 統合テストが旧バージョンを期待していたため修正: - checkout@v4 → checkout@v6 - setup-node@v4 → setup-node@v6 - create-pull-request@v6 → create-pull-request@v7 - codecov-action@v4 → codecov-action@v5 Co-authored-by: keito4 <keito4@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/integration/workflows.bats (1)
136-145:⚠️ Potential issue | 🟡 MinorLeftover
|| truerenders this security test a no-op.The PR objective explicitly calls out removing
|| truefrom bats tests to let failures surface, but line 143 was missed. With|| trueappended to the negated grep:
- If the grep matches (secret exposed in an
echo):grepexits 0 →!flips to failure →|| trueconverts back to success → test always passes.- If the grep does not match:
!yields success →|| trueshort-circuits → test also passes.This means a real exposure pattern will never be caught by this test.
🐛 Proposed fix
- ! grep -q 'echo.*\${{.*secrets\.' "$workflow" || true + ! grep -q 'echo.*\${{.*secrets\.' "$workflow"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/workflows.bats` around lines 136 - 145, The test "workflows do not expose secrets in environment variables" currently appends "|| true" to the negated grep which makes the check a no-op; update the test body in the bats function so the loop runs grep -q 'echo.*\${{.*secrets\.' "$workflow" without the trailing "|| true" (i.e., remove "|| true") so that a matching exposure causes grep to return 0, the negation "!" fails the test, and the test suite will surface failures as intended; ensure the test still iterates over "$workflows_dir"/*.yml and exits non-zero on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/integration/workflows.bats`:
- Around line 136-145: The test "workflows do not expose secrets in environment
variables" currently appends "|| true" to the negated grep which makes the check
a no-op; update the test body in the bats function so the loop runs grep -q
'echo.*\${{.*secrets\.' "$workflow" without the trailing "|| true" (i.e., remove
"|| true") so that a matching exposure causes grep to return 0, the negation "!"
fails the test, and the test suite will surface failures as intended; ensure the
test still iterates over "$workflows_dir"/*.yml and exits non-zero on failure.
- security-credential-scan.sh: (( )) を POSIX互換 $(( )) に置換 - security-credential-scan.sh: .claude/jsonl/lock.json/bats 除外、gitignore ファイルスキップ - security-credential-scan.sh: FINDINGS 配列の初期化修正（set -u 互換） - setup_claude.bats: リファクタリング後の実装に合わせてテスト更新 - development-tools.bats: 存在しないスクリプトのテスト削除、テスト条件緩和 - output.sh: 実行権限を付与 - pre_git_quality_gates.py: 失敗時の出力を末尾15行に制限（コンテキスト消費抑制） Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Claude Code レビューPR全体を確認しました。CIは主要チェックが全て通過しており、変更内容は適切です。以下に詳細フィードバックを記載します。 ✅ 良い点
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
.claude/hooks/pre_git_quality_gates.py (1)
180-190: LGTM — minor: simplify the redundant label/key tuple.The truncation logic is correct. The only nit is that
labelandkeyin each tuple are always identical, making the two-element tuple unnecessary.♻️ Simplify loop
- for label, key in [("stdout", "stdout"), ("stderr", "stderr")]: - output = failed.get(key, "").strip() + for key in ["stdout", "stderr"]: + output = failed.get(key, "").strip() if not output: continue lines = output.splitlines() if len(lines) <= MAX_LINES: - print(f" {label}:\n{output}", file=sys.stderr, flush=True) + print(f" {key}:\n{output}", file=sys.stderr, flush=True) else: truncated = "\n".join(lines[-MAX_LINES:]) - print(f" {label} (末尾{MAX_LINES}行 / 全{len(lines)}行):", file=sys.stderr, flush=True) + print(f" {key} (末尾{MAX_LINES}行 / 全{len(lines)}行):", file=sys.stderr, flush=True) print(truncated, file=sys.stderr, flush=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/pre_git_quality_gates.py around lines 180 - 190, The loop currently iterates over redundant (label, key) tuples where label == key; simplify it by iterating only over the keys (e.g., for key in ("stdout", "stderr")) and use label = key when printing. Update the block that references failed, MAX_LINES, lines, truncated and the print calls to use the single loop variable (key) and a label variable set to key so behavior and truncation logic remain identical but the tuple construction is removed.test/integration/setup_claude.bats (1)
67-70: Overly broadgreppattern forknown_marketplaces.The bare string
known_marketplacesmatches variable names, comments, or any incidental occurrence, not just the JSON filename reference. Pinning toknown_marketplaces.json(or whichever exact token is expected) prevents false positives from, e.g., a variable namedknown_marketplaces_path.♻️ Proposed fix
`@test` "setup-claude.sh references known_marketplaces" { # Verify known_marketplaces.json is used - grep -q 'known_marketplaces' "${REPO_ROOT}/script/setup-claude.sh" + grep -q 'known_marketplaces\.json' "${REPO_ROOT}/script/setup-claude.sh" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/setup_claude.bats` around lines 67 - 70, The test's grep is too broad—change the assertion in the "setup-claude.sh references known_marketplaces" test to match the exact filename token (e.g., known_marketplaces.json) instead of the bare string; update the grep invocation that currently searches "${REPO_ROOT}/script/setup-claude.sh" so it looks specifically for "known_marketplaces.json" (or a quoted exact token) to avoid matching variable names or comments.script/security-credential-scan.sh (3)
89-129: Consider removing directory entries fromEXCLUDE_PATTERNS— they were never effective
--exclude=<pattern>in grep matches file names only, not directory paths. Entries like"node_modules",".git","coverage","dist","build", and"__tests__"inEXCLUDE_PATTERNSwere always no-ops for directory exclusion. Now thatEXCLUDE_DIRSproperly handles these via--exclude-dir, the redundant entries can be removed to reduce confusion.♻️ Proposed cleanup of `EXCLUDE_PATTERNS`
EXCLUDE_PATTERNS=( - "node_modules" - ".git" - "coverage" - "dist" - "build" "*.min.js" "*.test.js" "*.test.ts" "*.spec.js" "*.spec.ts" - "__tests__" "*.example" "*.sample" "*.md" ".env.example" ".env.template" "*.jsonl" "*.lock.json" "*.bats" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/security-credential-scan.sh` around lines 89 - 129, Remove directory names from EXCLUDE_PATTERNS since grep's --exclude only matches file names; keep only file/glob patterns there and let EXCLUDE_DIRS handle directories. Update the EXCLUDE_PATTERNS array (symbol: EXCLUDE_PATTERNS) to drop entries like "node_modules", ".git", "coverage", "dist", "build", and "__tests__", and leave directory exclusions solely in EXCLUDE_DIRS (symbol: EXCLUDE_DIRS); adjust any comments if necessary and regenerate GREP_EXCLUDE (symbol: GREP_EXCLUDE) as currently done by the for-loops.
236-236: Inconsistent arithmetic style —((i++))retained while((CRITICAL_COUNT++))was converted
((i++))is safe here becauseistarts at1(never evaluates to0), but leaving it while all other((...))arithmetic was converted to$((var + 1))is inconsistent.♻️ Make consistent (applies to both lines 236 and 255)
- ((i++)) + i=$((i + 1))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/security-credential-scan.sh` at line 236, Replace the lone use of the C-style increment syntax for variable i with the consistent POSIX arithmetic substitution used elsewhere: change the increment expression ((i++)) to an assignment using $((...)) (e.g., set i to i + 1) and apply the same style consistency check to the CRITICAL_COUNT increment (CRITICAL_COUNT++ -> assign CRITICAL_COUNT to $((CRITICAL_COUNT + 1))) so both increments use the $((var + 1)) form; update occurrences around the i and CRITICAL_COUNT usages (the increment sites referenced in the review) accordingly.
148-151:git check-ignorespawned per finding — consider batching with--stdinEach grep match triggers a separate
git check-ignore -q "$file"subprocess. Across 9 patterns on a large repo this is O(patterns × matches) process spawns.git check-ignore --stdinaccepts multiple paths in one invocation, which would make a single pre-pass far cheaper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/security-credential-scan.sh` around lines 148 - 151, Replace per-match invocations of git check-ignore -q "$file" with a batched pre-pass: collect all candidate paths (the variable "$file" values from the grep matches) into a list or newline-separated stream, call git check-ignore --stdin once to get ignored paths, and then skip files by checking membership against that returned set; update the logic that currently uses git check-ignore -q "$file" to consult the precomputed ignored-paths set instead so you avoid spawning a git process for every match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/integration/development-tools.bats`:
- Around line 109-113: The third branch of the test assertion ([[ "$output" =~
"{" ]]) is too permissive and masks failures; remove that clause and tighten the
check so the test only accepts meaningful JSON indicators (keep the checks for
"critical_count" or "findings") or replace the weak regex with a proper JSON
validation (e.g., parse/validate the output) — locate the assertion in
test/integration/development-tools.bats where the three-way [[ "$output" =~
"critical_count" ]] || [[ "$output" =~ "findings" ]] || [[ "$output" =~ "{" ]]
is used and delete the final || [[ "$output" =~ "{" ]] branch or swap it for a
JSON validation step.
In `@test/integration/setup_claude.bats`:
- Around line 77-85: The two tests in setup_claude.bats are identical and
redundant; change them so each asserts a different plugins:: call to match their
intent — e.g., keep the "installation" test and grep for
"plugins::install_from_manifest" and change the "delegates plugin file parsing"
test to either be merged or to assert a different relevant function such as
"plugins::detect_and_add_marketplaces" or "plugins::copy_config_files" (or
remove/merge the second test if no distinct function exists); update the test
names/comments accordingly so one test verifies plugins::install_from_manifest
and the other verifies a distinct plugins::<function> call like
plugins::detect_and_add_marketplaces.
---
Nitpick comments:
In @.claude/hooks/pre_git_quality_gates.py:
- Around line 180-190: The loop currently iterates over redundant (label, key)
tuples where label == key; simplify it by iterating only over the keys (e.g.,
for key in ("stdout", "stderr")) and use label = key when printing. Update the
block that references failed, MAX_LINES, lines, truncated and the print calls to
use the single loop variable (key) and a label variable set to key so behavior
and truncation logic remain identical but the tuple construction is removed.
In `@script/security-credential-scan.sh`:
- Around line 89-129: Remove directory names from EXCLUDE_PATTERNS since grep's
--exclude only matches file names; keep only file/glob patterns there and let
EXCLUDE_DIRS handle directories. Update the EXCLUDE_PATTERNS array (symbol:
EXCLUDE_PATTERNS) to drop entries like "node_modules", ".git", "coverage",
"dist", "build", and "__tests__", and leave directory exclusions solely in
EXCLUDE_DIRS (symbol: EXCLUDE_DIRS); adjust any comments if necessary and
regenerate GREP_EXCLUDE (symbol: GREP_EXCLUDE) as currently done by the
for-loops.
- Line 236: Replace the lone use of the C-style increment syntax for variable i
with the consistent POSIX arithmetic substitution used elsewhere: change the
increment expression ((i++)) to an assignment using $((...)) (e.g., set i to i +
1) and apply the same style consistency check to the CRITICAL_COUNT increment
(CRITICAL_COUNT++ -> assign CRITICAL_COUNT to $((CRITICAL_COUNT + 1))) so both
increments use the $((var + 1)) form; update occurrences around the i and
CRITICAL_COUNT usages (the increment sites referenced in the review)
accordingly.
- Around line 148-151: Replace per-match invocations of git check-ignore -q
"$file" with a batched pre-pass: collect all candidate paths (the variable
"$file" values from the grep matches) into a list or newline-separated stream,
call git check-ignore --stdin once to get ignored paths, and then skip files by
checking membership against that returned set; update the logic that currently
uses git check-ignore -q "$file" to consult the precomputed ignored-paths set
instead so you avoid spawning a git process for every match.
In `@test/integration/setup_claude.bats`:
- Around line 67-70: The test's grep is too broad—change the assertion in the
"setup-claude.sh references known_marketplaces" test to match the exact filename
token (e.g., known_marketplaces.json) instead of the bare string; update the
grep invocation that currently searches "${REPO_ROOT}/script/setup-claude.sh" so
it looks specifically for "known_marketplaces.json" (or a quoted exact token) to
avoid matching variable names or comments.
| # Should complete the scan (may or may not find issues depending on environment) | ||
| [[ "$status" -eq 0 ]] || [[ "$status" -eq 1 ]] | ||
| # Output should be valid JSON | ||
| [[ "$output" =~ "critical_count" ]] | ||
| # Output should contain JSON structure (critical_count or findings) | ||
| [[ "$output" =~ "critical_count" ]] || [[ "$output" =~ "findings" ]] || [[ "$output" =~ "{" ]] | ||
| } |
There was a problem hiding this comment.
[[ "$output" =~ "{" ]] is an effectively vacuous assertion
The third || branch matches any output that contains a single {, including shell error messages. Since the test scaffolding creates a fresh git repo with no .gitignore, git check-ignore will never suppress test-file.js, so the script will always emit proper JSON with critical_count. The first condition is the only meaningful guard; the third condition just hides failures.
🛡️ Tighten the assertion
- # Output should contain JSON structure (critical_count or findings)
- [[ "$output" =~ "critical_count" ]] || [[ "$output" =~ "findings" ]] || [[ "$output" =~ "{" ]]
+ # Output should contain valid JSON with critical_count key
+ [[ "$output" =~ "critical_count" ]]📝 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.
| # Should complete the scan (may or may not find issues depending on environment) | |
| [[ "$status" -eq 0 ]] || [[ "$status" -eq 1 ]] | |
| # Output should be valid JSON | |
| [[ "$output" =~ "critical_count" ]] | |
| # Output should contain JSON structure (critical_count or findings) | |
| [[ "$output" =~ "critical_count" ]] || [[ "$output" =~ "findings" ]] || [[ "$output" =~ "{" ]] | |
| } | |
| # Should complete the scan (may or may not find issues depending on environment) | |
| [[ "$status" -eq 0 ]] || [[ "$status" -eq 1 ]] | |
| # Output should contain valid JSON with critical_count key | |
| [[ "$output" =~ "critical_count" ]] | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/integration/development-tools.bats` around lines 109 - 113, The third
branch of the test assertion ([[ "$output" =~ "{" ]]) is too permissive and
masks failures; remove that clause and tighten the check so the test only
accepts meaningful JSON indicators (keep the checks for "critical_count" or
"findings") or replace the weak regex with a proper JSON validation (e.g.,
parse/validate the output) — locate the assertion in
test/integration/development-tools.bats where the three-way [[ "$output" =~
"critical_count" ]] || [[ "$output" =~ "findings" ]] || [[ "$output" =~ "{" ]]
is used and delete the final || [[ "$output" =~ "{" ]] branch or swap it for a
JSON validation step.
| @test "setup-claude.sh calls plugins library for installation" { | ||
| # Verify plugin installation is delegated to claude_plugins.sh | ||
| grep -q 'plugins::' "${REPO_ROOT}/script/setup-claude.sh" | ||
| } | ||
|
|
||
| @test "setup-claude.sh skips comments and empty lines" { | ||
| # Verify comment/empty line skipping | ||
| grep -q '\[\[ -z "$line" \|\| "$line" =~ .*# \]\]' "${REPO_ROOT}/script/setup-claude.sh" | ||
| @test "setup-claude.sh delegates plugin file parsing to library" { | ||
| # Comment/empty line skipping is handled by claude_plugins.sh | ||
| grep -q 'plugins::' "${REPO_ROOT}/script/setup-claude.sh" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'plugins::' --type sh "${REPO_ROOT}/script/setup-claude.sh" 2>/dev/null || rg -n 'plugins::' setup-claude.sh 2>/dev/null || find . -name setup-claude.sh -type f | head -5Repository: keito4/config
Length of output: 81
🏁 Script executed:
find . -name "setup-claude.sh" -type f 2>/dev/nullRepository: keito4/config
Length of output: 81
🏁 Script executed:
rg -n 'plugins::' ./script/setup-claude.shRepository: keito4/config
Length of output: 373
🏁 Script executed:
cat -n ./test/integration/setup_claude.bats | sed -n '77,85p'Repository: keito4/config
Length of output: 505
🏁 Script executed:
cat -n ./script/setup-claude.sh | sed -n '75,105p'Repository: keito4/config
Length of output: 1020
Duplicate grep in two distinct tests makes one of them vacuous.
Both "setup-claude.sh calls plugins library for installation" (lines 77–80) and "setup-claude.sh delegates plugin file parsing to library" (lines 82–85) execute the identical assertion:
grep -q 'plugins::' "${REPO_ROOT}/script/setup-claude.sh"Because the two tests are indistinguishable, a failure in either surfaces as both failing together, and a pass in one guarantees a pass in the other — adding zero test coverage for the second scenario. Either merge them into a single test or make each grep distinct enough to cover its stated intent.
The script contains these distinct plugins:: function calls:
plugins::sync_repo_contentplugins::copy_config_filesplugins::detect_and_add_marketplacesplugins::install_from_manifest(line 98 — aligns with "installation" test)plugins::apply_hookify_patch
Differentiate by targeting specific functions. For the first test, plugins::install_from_manifest is appropriate. For the second test, clarify the actual intent — the comment mentions "Comment/empty line skipping" but no dedicated parse function exists; consider whether this should check for a specific function or be merged with the first test.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/integration/setup_claude.bats` around lines 77 - 85, The two tests in
setup_claude.bats are identical and redundant; change them so each asserts a
different plugins:: call to match their intent — e.g., keep the "installation"
test and grep for "plugins::install_from_manifest" and change the "delegates
plugin file parsing" test to either be merged or to assert a different relevant
function such as "plugins::detect_and_add_marketplaces" or
"plugins::copy_config_files" (or remove/merge the second test if no distinct
function exists); update the test names/comments accordingly so one test
verifies plugins::install_from_manifest and the other verifies a distinct
plugins::<function> call like plugins::detect_and_add_marketplaces.
|
🎉 This PR is included in version 1.76.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
CLAUDE_CODE_OAUTH_TOKENを削除（docker historyでのシークレット露出を防止、secrets mount のみ使用）|| trueを削除しテスト失敗を検出可能に。Slack 通知のpayload-file-pathをpayload-template-file-pathに修正（${{ env.* }}変数展開が正しく動作するように）create-pull-request@v6→@v7、ドキュメント内のcheckout@v4→@v6/lib/全体除外から zsh スクリプト4件のみ除外に変更（claude_plugins.shが新たにチェック対象に）update-actions.shのsed -i ''をsed -i.bakに変更（Linux 対応）.trivyignoreの全5件にレビュー日を追加check-docs-sync.sh（参照するnpm run docs:generateが不存在）、docs-common.js（未使用）を削除し、関連ドキュメント参照もクリーンアップTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Bug Fixes
Tests
Chores