Skip to content

fix: update Claude Code settings#146

Merged
Wintus merged 3 commits intotrunkfrom
fix/claude-settings
Jan 5, 2026
Merged

fix: update Claude Code settings#146
Wintus merged 3 commits intotrunkfrom
fix/claude-settings

Conversation

@Wintus
Copy link
Owner

@Wintus Wintus commented Jan 5, 2026

Summary

  • Rename settings.local.json to settings.json for version control
  • Add gh run list/view permissions
  • Remove redundant allow entries (checkout, --set-upstream)

Co-Authored-By: Claude

Wintus added 3 commits January 5, 2026 20:47
Use the shared settings file format instead of local-only
to track Claude Code permissions in version control.

Co-Authored-By: Claude
Allow listing and viewing GitHub Actions workflow runs.

Co-Authored-By: Claude
- Remove git push --set-upstream (keep -u)
- Remove git checkout (keep switch per project policy)

Co-Authored-By: Claude
@Wintus Wintus self-assigned this Jan 5, 2026
@claude
Copy link

claude bot commented Jan 5, 2026

Code Review: PR #146 - Update Claude Code Settings

Overview

This PR renames settings.local.json to settings.json and makes minor permission updates. The changes are minimal and focused, which is good.

✅ Positive Aspects

  1. Good cleanup: Removing redundant permission entries (git checkout, git push --set-upstream) simplifies the configuration
  2. Useful additions: Adding gh run list/view permissions enables better CI/CD workflow visibility
  3. Clean changes: The diff is minimal and easy to review

⚠️ Concerns & Recommendations

1. Version Control Strategy (Important)

The file is being renamed from settings.local.json to settings.json. Based on the .gitignore file, *.local files are excluded from version control (line 13), which suggests:

  • settings.local.json was intended to be user-specific and not committed
  • settings.json will be committed and shared across all users

Questions to consider:

  • Is this intentional? Do you want all team members to use the same Claude Code permissions?
  • If these are meant to be personal settings, you should revert this and keep the .local suffix
  • If these are meant to be team-wide defaults, this change is correct, but consider:
    • Adding .claude/settings.local.json to .gitignore explicitly (for user overrides)
    • Documenting this in the project README or CLAUDE.md

Recommendation: Add a note to CLAUDE.md explaining:

## Claude Code Settings

The `.claude/settings.json` file contains default permissions for Claude Code.
Users can create `.claude/settings.local.json` to override these settings locally.

2. Missing Permissions Audit

While the additions are helpful, consider if these permissions are also needed (based on common GitHub CLI usage):

  • Bash(gh issue view:*) - viewing individual issues (you have gh issue:* which already covers this)
  • Bash(gh search:*) - searching across the repository
  • Bash(gh pr comment:*) - commenting on PRs (useful for automated reviews)
  • Bash(gh pr diff:*) - viewing PR diffs
  • Bash(gh pr view:*) - viewing PR details

Actually, looking at the current settings, Bash(gh pr:*) and Bash(gh issue:*) already cover these with wildcards, so this is fine.

3. Consistency Check

The removed entries:

  • Bash(git checkout:*) - Reasonable to remove since git switch is the modern alternative
  • Bash(git push --set-upstream:*) - Redundant with Bash(git push -u:*)

This is good cleanup.

📋 Test Coverage

Not applicable - this is a configuration file change with no runtime impact on the application code.

🔒 Security

The permission changes are appropriate:

  • No new security risks introduced
  • Deny list remains intact with dangerous operations still blocked
  • Ask list appropriately prompts for package management operations

🎯 Final Verdict

Conditional Approval - The code changes are good, but please clarify the version control strategy:

  1. If team-wide settings: Approve and merge, but document in CLAUDE.md
  2. If personal settings: Revert the rename and keep as settings.local.json

The permission updates themselves are sound and well-reasoned.


Review generated by Claude Code

@Wintus Wintus merged commit 37c048d into trunk Jan 5, 2026
4 checks passed
@Wintus Wintus deleted the fix/claude-settings branch January 5, 2026 14:06
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.

1 participant