-
Notifications
You must be signed in to change notification settings - Fork 668
NO-JIRA: Add Claude Code settings configuration #15819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NO-JIRA: Add Claude Code settings configuration #15819
Conversation
WalkthroughAdds a new JSON configuration file Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Comment |
|
/test e2e-gcp-console |
1 similar comment
|
/test e2e-gcp-console |
|
@sg00dwin: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest-required |
jhadvig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding couple of comments. @sg00dwin please review for which command are using the semicolon syntax since git, oc, .... are not
.claude/settings.json
Outdated
| "Bash(git diff:*)", | ||
| "Bash(git log:*)", | ||
| "Bash(git branch:*)", | ||
| "Bash(git show:*)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lest also add git stash * & git remove *
.claude/settings.json
Outdated
|
|
||
| "Bash(git stash drop:*)", | ||
| "Bash(git stash clear:*)", | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add:
"Bash(git revert --no-commit *)",
"Bash(git branch -D *)",
"Bash(git branch --delete --force *)",
| "Bash(gh pr diff:*)", | ||
| "Bash(gh pr checks:*)", | ||
|
|
||
| "WebFetch(domain:github.com)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add:
"WebFetch(domain:docs.openshift.com)",
"WebFetch(domain:kubernetes.io)",
.claude/settings.json
Outdated
| "Bash(yarn lint:*)", | ||
| "Bash(yarn lint)", | ||
| "Bash(yarn eslint:*)", | ||
| "Bash(yarn gherkin-lint:*)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add:
"Bash(yarn prettier *)",
"Bash(yarn tsc)",
"Bash(yarn type-check)",
.claude/settings.json
Outdated
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash(yarn test:*)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are not using yarn command with the semi colon
.claude/settings.json
Outdated
|
|
||
| "Bash(tree:*)", | ||
|
|
||
| "Bash(git fetch:*)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git cmd down not used subcommands with semicolon, meaning there is no git diff:HEAD only git diff HEAD
Configures permissions for Claude Code AI assistant with allow/deny/ask rules for common development commands including testing, linting, git operations, and cluster management. Assisted by Claude Code
31d648c to
aca4c0d
Compare
|
/label tide/merge-method-squash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @.claude/settings.json:
- Line 33: The entry "Bash(git remove *)" is using an invalid Git command;
update that settings entry to use the correct command "git rm" (e.g., change the
string "Bash(git remove *)" to "Bash(git rm *)") so the configured Bash action
invokes the valid git rm command.
🧹 Nitpick comments (2)
.claude/settings.json (2)
66-101: Consider adding additional destructive operations to deny list.The deny list covers most critical destructive operations, but consider adding:
- Docker cleanup:
Bash(docker system prune *)andBash(docker volume prune *)- Package removal:
Bash(yarn remove *)andBash(npm uninstall *)These operations can have significant impact and may warrant explicit denial or moving to the ask list.
3-130: Consider consistent wildcard usage patterns.Some commands have both wildcard and non-wildcard versions (e.g.,
yarn test *andyarn test), while others only have one form. Consider establishing a consistent pattern:
- Use both forms when commands can accept arguments (e.g.,
yarn test unit.test.js)- Use only non-wildcard for commands that don't accept arguments (e.g.,
git status)This improves maintainability and makes the permission rules clearer.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.claude/settings.json
🔇 Additional comments (1)
.claude/settings.json (1)
30-32: No action needed. Deny rules take precedence over allow wildcards in Claude Code's permission system, sogit branch *andgit stash *in the allow list do not inadvertently permit the destructive operations blocked in the deny list (git branch -D,git stash drop,git stash clear).Likely an incorrect or invalid review comment.
.claude/settings.json
Outdated
| "Bash(git branch *)", | ||
| "Bash(git show *)", | ||
| "Bash(git stash *)", | ||
| "Bash(git remove *)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid git command.
The command git remove does not exist. The correct command is git rm for removing files from the working tree and index.
🔎 Proposed fix
- "Bash(git remove *)",
+ "Bash(git rm *)",📝 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.
| "Bash(git remove *)", | |
| "Bash(git rm *)", |
🤖 Prompt for AI Agents
In @.claude/settings.json around line 33, The entry "Bash(git remove *)" is
using an invalid Git command; update that settings entry to use the correct
command "git rm" (e.g., change the string "Bash(git remove *)" to "Bash(git rm
*)") so the configured Bash action invokes the valid git rm command.
.claude/settings.json
Outdated
| "Bash(git branch *)", | ||
| "Bash(git show *)", | ||
| "Bash(git stash *)", | ||
| "Bash(git remove *)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be:
"Bash(git remote *)",
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.claude/settings.json (3)
3-64: Well-structured allow list with appropriate safe commands.The allow list properly covers read-only and non-destructive operations. Good incorporation of past feedback including
git stash *,git remote *, yarn linting tools, and documentation domains.Optional improvement: Consider adding
git pullto theasklist since it can modify the working directory and merge changes, which might warrant user confirmation.
66-101: Strengthen the deny list with additional destructive commands.The deny list appropriately blocks dangerous operations including force push, hard reset, and cluster modifications. Past feedback regarding
git revert --no-commit,git branch -D, and force deletion has been properly incorporated.Consider adding these destructive commands to the deny list:
"Bash(git rm *)"- removes files from working tree and index"Bash(rm -r *)"- recursive removal without force flag is still destructive🔎 Suggested additions
"Bash(git branch --delete --force *)", + + "Bash(git rm *)", + "Bash(git rm -r *)", + "Bash(git rm -rf *)", "Bash(rm -rf *)", "Bash(rm -fr *)", + "Bash(rm -r *)",
103-130: Enhance the ask list with package removal commands.The ask list appropriately requires confirmation for installs, git write operations, PR management, and builds.
Consider adding package removal commands for consistency:
"Bash(yarn remove *)""Bash(npm uninstall *)""Bash(npm remove *)""Bash(npm rm *)"These commands modify dependencies similar to install/add operations and should warrant user confirmation.
🔎 Suggested additions
"Bash(npm install *)", "Bash(npm i *)", + "Bash(yarn remove *)", + "Bash(npm uninstall *)", + "Bash(npm remove *)", + "Bash(npm rm *)", "Bash(go get *)", "Bash(go mod *)",
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.claude/settings.json
🔇 Additional comments (1)
.claude/settings.json (1)
1-132: Configuration successfully addresses past feedback and achieves PR objectives.The Claude Code settings configuration is well-structured with clear separation of allow/deny/ask permissions. Key improvements from previous reviews have been incorporated:
- ✅ Git commands corrected (
git remoteinstead ofgit remove)- ✅ Destructive git operations added to deny list
- ✅ Documentation domains added to WebFetch
- ✅ Additional yarn linting tools included
The JSON structure is valid and the permission sets appropriately cover testing, linting, git operations, and cluster management as described in the PR objectives.
jhadvig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jhadvig, sg00dwin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Overriding since this PR is only adding AI settings. |
|
@jhadvig: Overrode contexts on behalf of jhadvig: ci/prow/e2e-gcp-console, ci/prow/okd-scos-images DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/override ci/prow/okd-scos-images |
|
@jhadvig: Overrode contexts on behalf of jhadvig: ci/prow/okd-scos-images DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Overriding since we are only adding AI settings in this PR. |
|
@jhadvig: Overrode contexts on behalf of jhadvig: ci/prow/analyze, ci/prow/backend, ci/prow/e2e-gcp-console, ci/prow/frontend, ci/prow/images DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Overriding the TC since we are only adding a AI config file |
|
@jhadvig: Overrode contexts on behalf of jhadvig: ci/prow/backend DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@sg00dwin: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Configures permissions for Claude Code AI assistant with allow/deny/ask rules for common development commands including testing, linting, git operations, and cluster management.