Skip to content

Conversation

@chicks-net
Copy link
Member

Done

  • 🔸 [claude] allow just shellcheck

Meta

(Automated in .just/gh-process.just.)

Copilot AI review requested due to automatic review settings November 23, 2025 03:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds permission for Claude to execute just shellcheck commands by updating the allowed bash commands in the Claude settings configuration. The change also reorganizes the order of some existing permissions.

  • Added Bash(just shellcheck:*) to the list of allowed commands
  • Reordered existing permissions for better organization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@claude
Copy link

claude bot commented Nov 23, 2025

Code Review

Summary

This PR adds Bash(just shellcheck:*) to the allowed permissions in Claude Code settings, enabling Claude to run shellcheck validation on just recipes without prompting for permission. This aligns with the repository's shellcheck integration added in v3.8bis.

✅ Positive Findings

1. Appropriate Permission Scope

  • The permission Bash(just shellcheck:*) is properly scoped to only allow the specific just shellcheck command
  • Uses wildcard pattern to allow any arguments to the shellcheck recipe
  • Consistent with existing permission patterns (e.g., Bash(gh pr view:*))

2. Alphabetical Organization Improvement

  • The reordering improves logical grouping:
    • General tools first (WebFetch, WebSearch)
    • Linting tools together (markdownlint-cli2, then later shellcheck)
    • GitHub CLI tools grouped together
  • However, note the alphabetical ordering isn't perfectly consistent (see suggestion below)

3. Alignment with Repository Workflow

💡 Suggestions for Improvement

Minor: Consider Full Alphabetical Consistency

While the grouping makes logical sense, the WebSearch line was moved up but the overall order could be more consistent. Consider either:

  1. Strict alphabetical (current approach is fine)
  2. Or group by tool type with comments:
{
  "permissions": {
    "allow": [
      // Web tools
      "WebFetch(domain:www.chicks.net)",
      "WebFetch(domain:developers.facebook.com)",
      "WebFetch(domain:github.com)",
      "WebFetch(domain:raw.githubusercontent.com)",
      "WebFetch(domain:en.wikipedia.org)",
      "WebSearch",
      
      // Linting tools
      "Bash(markdownlint-cli2:*)",
      "Bash(just shellcheck:*)",
      
      // GitHub CLI
      "Bash(gh pr view:*)",
      "Bash(gh pr diff:*)",
      
      // Git
      "Bash(git log:*)",
      
      // Other
      "Bash(identify:*)"
    ],
    "deny": [],
    "ask": []
  }
}

Note: This is purely aesthetic - the current implementation is functionally perfect.

🔒 Security Assessment

No security concerns. The permission is:

  • Read-only operation (shellcheck only analyzes code)
  • Scoped to a specific command
  • Runs on locally checked-out code
  • Consistent with the principle of least privilege

📊 Overall Assessment

Verdict: ✅ LGTM (Looks Good To Me)

This is a clean, focused change that:

  • Has a clear purpose
  • Follows existing patterns
  • Improves Claude's ability to autonomously maintain code quality
  • Introduces no security risks
  • Aligns with repository best practices

The minor reordering is actually an improvement for readability. This PR is ready to merge.


Review performed by Claude Code

@chicks-net chicks-net merged commit db31533 into main Nov 23, 2025
6 checks passed
@chicks-net chicks-net deleted the chicks/2025-11-23-claude-settings branch November 23, 2025 03:48
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