[simonheimlicher]

Summary

• Add use_project_plugins boolean input (default false) to both reusable workflows. Gates the read of the caller's .claude/settings.json. When off, neither enabledPlugins nor extraKnownMarketplaces flow into the action — the run uses only the wrapper inputs (extra_plugins, plugin_marketplaces). When the file is present and the flag is off, a ::notice:: tells the caller how to opt in.
• Bump anthropics/claude-code-action from v1.0.111 to v1.0.123 (pinned SHA), picking up upstream PR #1312 (bump Bun to 1.3.14) which fixes the "Internal error: directory mismatch ... tsconfig.json, fd 4" lines that appear at the end of every run on v1.0.111.

Motivation

A real review run (leoherds/leoherd#157) spent 8 minutes, ran 21 turns, and hit 13 permission denials before exiting without posting a comment. Root cause: the caller's .claude/settings.json declared 6 plugins, and several of those plugins' skills carry "ALWAYS invoke this skill before X" directives that demand tools (Read, Grep, Glob, Bash(git ...)) outside the review wrapper's gh-only allowlist.

Defaulting the project-plugin install to off makes the review and mention jobs match the wrapper's narrow tool surface out of the box. Callers who want plugin skills active opt in and widen the allowlist together.

Test plan

• Open a PR on a downstream consumer repo without setting use_project_plugins; confirm the workflow installs no plugins, emits the ::notice::, and posts a review comment.
• Open a PR with use_project_plugins: true plus append_allow_list: "Read,Grep,Glob,Bash(git:*)"; confirm plugins install and the review still completes.
• Confirm the "Internal error: directory mismatch ... tsconfig.json, fd 4" lines no longer appear in the run log after the bump.
• Verify both example caller workflows still copy-paste cleanly into a downstream repo.

[claude]

Code Review

Overview

This PR makes two independent improvements:

1. use_project_plugins flag (default false) — gates whether .claude/settings.json plugins/marketplaces flow into the action. Motivated by a real incident where plugin skill auto-invocation mandates burned all available turns on permission denials before any review comment was posted.
2. Action bump v1.0.111 → v1.0.123 (pinned SHA) — picks up a Bun upgrade that fixes spurious tsconfig.json / fd 4 error lines in run logs.

Both changes are well-reasoned and the PR description clearly documents the root cause. The implementation is solid. A few observations below.

---

What works well

• Safe default. Defaulting use_project_plugins to false is the right call. The old behaviour silently picked up project plugins whose skills could subvert the wrapper's narrow gh-only tool surface; this makes the surface explicit and opt-in.
• ::notice:: guidance. Emitting a notice when .claude/settings.json is present but the flag is off gives operators a clear signal without failing the run. Good UX.
• set -euo pipefail is preserved in both shell blocks — no regressions on error propagation.
• README and example comments are in sync with the implementation.

---

Issues / suggestions

1. Boolean env-var comparison is safe but fragile across YAML types

USE_PROJECT_PLUGINS: ${{ inputs.use_project_plugins }}

if [[ "$USE_PROJECT_PLUGINS" == "true" ]]; then

GitHub Actions serialises a boolean input to the string "true" or "false" when you reference it in env:, so the comparison works today. This is fine, but worth a comment (or consider == 'true' lowercase with a note) so a future editor doesn't second-guess it.

2. Code duplication between the two workflows

The entire plugin-resolution shell block is now copy-pasted verbatim between claude.yml and claude-code-review.yml. Any future change (e.g. supporting source.source: "gitlab") needs to land in both files. This is a pre-existing pattern in the repo and acceptable for YAML workflows, but it's worth tracking in an issue so it doesn't get out of sync long-term.

3. Action SHA should be verified

The bump from fefa07e9c665b7320f08c3b525980457f22f58aa to 51ea8ea73a139f2a74ff649e3092c25a904aed7e is presented as v1.0.123. Confirming that the SHA actually corresponds to the tagged release (rather than an arbitrary commit) before merge is a good supply-chain hygiene step. A quick gh api repos/anthropics/claude-code-action/git/ref/tags/v1.0.123 can validate this.

4. Notice is only emitted when use_project_plugins == false and the file exists

elif [[ -f .claude/settings.json ]]; then
    echo "::notice::..."
fi

This is the intended behaviour, but it means callers that never checked in a .claude/settings.json get no feedback about the new flag's existence. The README update handles this for first-time readers, so this is not a bug — just worth being aware of in support cases.

5. Minor: jq empty is used as a JSON-validity gate

jq empty .claude/settings.json

jq empty exits non-zero on invalid JSON, which (combined with set -e) aborts the step cleanly. Good defensive programming. One edge case: if .claude/settings.json is syntactically valid JSON but has an unexpected schema (e.g. enabledPlugins is an array instead of an object), the downstream jq expressions will produce empty output silently rather than erroring. This is unlikely in practice and acceptable for now, but a stricter schema check could be added later.

---

Security

The change is a net improvement: project plugins are no longer silently installed in an environment where the tool allowlist was not designed for them. The new README security section is clearer and adds the self-hosted runner warning, which is valuable.

No new attack surface is introduced. The ::notice:: message doesn't leak secrets. The flag is a boolean input with a safe default.

---

Verdict

Approve with minor suggestions. The logic is correct, the default is safe, the documentation is thorough, and the action bump is unrelated but clearly beneficial. The main ask before merge is verifying the SHA for v1.0.123 against the upstream tag. The code-duplication point is cosmetic and can be an issue rather than a blocker.

[simonheimlicher]

Thanks for the review. Addressing the one merge-gating point:

SHA verification for v1.0.123 — confirmed via the annotated tag chain:

$ gh api repos/anthropics/claude-code-action/git/refs/tags/v1.0.123 --jq '.object'
{"sha":"d2b7d924ec4af238380e490a0d57ce6bcc85ce8d","type":"tag",...}

$ gh api repos/anthropics/claude-code-action/git/tags/d2b7d924ec4af238380e490a0d57ce6bcc85ce8d --jq '{commit_sha: .object.sha, tagger: .tagger.name, message: .message}'
{"commit_sha":"51ea8ea73a139f2a74ff649e3092c25a904aed7e","tagger":"github-actions[bot]","message":"Release v1.0.123\n"}

The pinned SHA 51ea8ea73a139f2a74ff649e3092c25a904aed7e matches the commit that the v1.0.123 annotated tag points to, with the canonical release tagger.

The other points (env-var stringification, workflow code duplication, notice-only-when-file-exists, jq schema strictness) are acknowledged as non-blocking. Merging via rebase to preserve the two-commit split.

[claude]

Review (post-merge)

This PR is already merged and has an excellent existing review thread, so I'll keep this brief.

The changes are sound. The use_project_plugins: false default solves a real failure mode — plugin skills with ALWAYS invoke directives consuming turns against a narrow gh-only allowlist until the run exits without posting anything. The implementation is clean and the SHA pin for the action bump follows security best practices.

One additional observation not in the existing review

No feedback when use_project_plugins: true but .claude/settings.json is absent. The ::notice:: fires in the elif branch (file present, flag false), but there's no corresponding notice in the opposite case (flag true, file absent). A caller who sets use_project_plugins: true expecting plugins to be loaded from a file that doesn't exist will get a silent no-op. Low urgency since the README makes the precondition clear, but a one-liner notice would make the misconfig easier to spot in run logs:

if [[ "$USE_PROJECT_PLUGINS" == "true" ]]; then
  if [[ -f .claude/settings.json ]]; then
    # ... existing parse logic ...
  else
    echo "::notice::use_project_plugins is true but .claude/settings.json was not found; no project plugins will be installed."
  fi
elif [[ -f .claude/settings.json ]]; then
  echo "::notice::..."  # existing notice
fi

Worth filing as a small follow-up issue rather than reopening this PR.

---

Overall: the default-off approach is the right design, the implementation is correct, and the documentation improvements (README section rename, example workflow comments) are clear and actionable. Good work.