Repository navigation
feat: Add support for Claude customization (inference backend, skills, etc) - #116
Merged
Merged
Conversation
…, mcp, etc) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR migrates Claude Code skills discovery from ChangesSkills Path Migration and Multi-Backend Deployment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
agents/claude-code/deployment/README.md (1)
370-381: ⚡ Quick winAdd inline YAML examples for manual configuration steps.
The instructions tell users to "Edit
deployment-vllm.yamland update" several environment variables, but don't show where these fields appear in the YAML structure. Users unfamiliar with the manifest will need to search through the file.Consider adding a brief YAML snippet showing the env section structure for these variables to reduce friction.
Example inline snippet
Edit `deployment-vllm.yaml` and update: - `ANTHROPIC_BASE_URL`: Your vLLM server URL (e.g., `https://vllm.apps.cluster.domain`) - `ANTHROPIC_CUSTOM_MODEL_OPTION`: Your model ID (bare model name, no prefix) - Model alias overrides (required to prevent 404 errors): - `ANTHROPIC_DEFAULT_HAIKU_MODEL`: Set to your vLLM model ID - `ANTHROPIC_DEFAULT_SONNET_MODEL`: Set to your vLLM model ID - `ANTHROPIC_DEFAULT_OPUS_MODEL`: Set to your vLLM model ID - `claude-vllm-settings` ConfigMap: Set the `model` field to your model ID + +Example environment variable section in the Deployment: +```yaml +env: + - name: ANTHROPIC_BASE_URL + value: "https://vllm.apps.cluster.domain" + - name: ANTHROPIC_CUSTOM_MODEL_OPTION + value: "your-model-id" + - name: ANTHROPIC_DEFAULT_HAIKU_MODEL + value: "your-model-id" + - name: ANTHROPIC_DEFAULT_SONNET_MODEL + value: "your-model-id" + - name: ANTHROPIC_DEFAULT_OPUS_MODEL + value: "your-model-id" +```🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/claude-code/deployment/README.md` around lines 370 - 381, Add an inline YAML env snippet to README's "Update the deployment manifest" section showing where to set ANTHROPIC_BASE_URL, ANTHROPIC_CUSTOM_MODEL_OPTION and the three ANTHROPIC_DEFAULT_* variables; include a short example of the container env: block with - name: ANTHROPIC_BASE_URL / value: "https://vllm.apps.cluster.domain", - name: ANTHROPIC_CUSTOM_MODEL_OPTION / value: "your-model-id", and the three - name: ANTHROPIC_DEFAULT_HAIKU_MODEL, ANTHROPIC_DEFAULT_SONNET_MODEL, ANTHROPIC_DEFAULT_OPUS_MODEL entries so readers can see the exact YAML structure to edit in deployment-vllm.yaml and the claude-vllm-settings ConfigMap.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/claude-code/deployment/README.md`:
- Line 506: The README's "OGX with PostgreSQL" note is too vague; add a short
PostgreSQL deployment subsection under that heading explaining how to satisfy
the prerequisite: include links to OGX/RHOAI official docs, a minimal example
manifest (Kubernetes StatefulSet/Deployment + Service) or Helm chart command,
and an optional Docker Compose snippet for local testing; reference the README
section title "OGX with PostgreSQL" so the new content sits directly beneath it
and mention common connection parameters (host, port, user, password, db name)
and a note about creating the required database/schema before starting OGX.
- Line 269: The README note mentions "Vertex AI (and Bedrock/Foundry)" which are
not documented elsewhere; update the sentence referencing the model alias
`sonnet` and the `CLAUDE_MODEL` env var to remove the undocumented
"Bedrock/Foundry" platforms (or alternatively add corresponding deployment
docs/manifests for Bedrock and Foundry). Specifically edit the paragraph that
mentions `sonnet` and `CLAUDE_MODEL` so it only references platforms actually
covered (Anthropic API, Vertex AI, vLLM direct, OGX) or add links/sections for
Bedrock/Foundry if you opt to document them.
- Around line 719-738: Add a short security guidance section near the mcpServers
example explaining that placeholders like ${API_TOKEN} must not be hardcoded in
the ConfigMap JSON and should be injected via Kubernetes Secrets; mention the
mcpServers -> remote-api -> headers -> "Authorization" example and instruct
implementers to mount or inject the secret as an environment variable (e.g.,
API_TOKEN) from a Secret in their Deployment/Pod spec (valueFrom secretKeyRef)
or reference it via an external secret management tool, and explicitly call out
to avoid placing credentials in the ConfigMap.
---
Nitpick comments:
In `@agents/claude-code/deployment/README.md`:
- Around line 370-381: Add an inline YAML env snippet to README's "Update the
deployment manifest" section showing where to set ANTHROPIC_BASE_URL,
ANTHROPIC_CUSTOM_MODEL_OPTION and the three ANTHROPIC_DEFAULT_* variables;
include a short example of the container env: block with - name:
ANTHROPIC_BASE_URL / value: "https://vllm.apps.cluster.domain", - name:
ANTHROPIC_CUSTOM_MODEL_OPTION / value: "your-model-id", and the three - name:
ANTHROPIC_DEFAULT_HAIKU_MODEL, ANTHROPIC_DEFAULT_SONNET_MODEL,
ANTHROPIC_DEFAULT_OPUS_MODEL entries so readers can see the exact YAML structure
to edit in deployment-vllm.yaml and the claude-vllm-settings ConfigMap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: cbf160af-cd4d-4401-8a9a-903d124f403e
📒 Files selected for processing (7)
agents/claude-code/deployment/Containerfileagents/claude-code/deployment/README.mdagents/claude-code/deployment/deployment-ogx-vllm.yamlagents/claude-code/deployment/deployment-vertex.yamlagents/claude-code/deployment/deployment-vllm.yamlagents/claude-code/deployment/deployment.yamlagents/claude-code/deployment/entrypoint.sh
andrewdonheiser
requested changes
Jun 5, 2026
andrewdonheiser
left a comment
andrewdonheiser
left a comment
Contributor
There was a problem hiding this comment.
Review Summary
Good work on the skills path fix and the new vLLM/OGX deployment options. Security context is solid across all manifests, and the entrypoint.sh cleanup is clean. A few issues need addressing before merge — the most critical being missing env vars in the vLLM manifest that will cause runtime 404s.
Critical (3)
- Missing
ANTHROPIC_DEFAULT_*_MODELoverrides indeployment-vllm.yaml— will 404 at runtime - Context window size comment says ~14K in vLLM manifest but ~23K everywhere else
- "Note on model selection" paragraph placed under wrong heading in Vertex section
Important (5)
- README step ordering: "Before deploying" advice appears after deployment steps
- Skills ConfigMap
--from-fileexample uses invalid keys (slashes) claude-settingsvolume missingoptional: trueacross all manifests- Customization section only references Option A ConfigMap names
SKIP_PERMISSIONSwarning text inconsistent across manifests
See inline comments for details and suggested fixes.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/claude-code/deployment/README.md`:
- Around line 705-712: The README example hardcodes the ConfigMap name
"claude-skills" which doesn't match backend-specific mounts (for example the
vLLM manifest mounts "claude-vllm-skills" in deployment-vllm.yaml), so update
the example command to either use a placeholder/name variable or show the
correct name per deployment option and add a short note listing the valid skills
ConfigMap names per option (e.g., mention "claude-vllm-skills" for vLLM and the
default name used by the other deployment), and remind users to update the
volume spec's items entries to match the chosen ConfigMap when adding skills
(affecting the skills volume items projection).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 0492735b-bcb2-4509-b374-cc4b77996061
📒 Files selected for processing (5)
agents/claude-code/deployment/README.mdagents/claude-code/deployment/deployment-ogx-vllm.yamlagents/claude-code/deployment/deployment-vertex.yamlagents/claude-code/deployment/deployment-vllm.yamlagents/claude-code/deployment/deployment.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- agents/claude-code/deployment/deployment.yaml
- agents/claude-code/deployment/deployment-vertex.yaml
- agents/claude-code/deployment/deployment-ogx-vllm.yaml
Comment on lines
+705
to
+712
| # Create a ConfigMap with skills | ||
| oc create configmap claude-skills \ | ||
| --from-file=code-review-skill=./skills/code-review/SKILL.md \ | ||
| --from-file=security-audit-skill=./skills/security-audit/SKILL.md | ||
| ``` | ||
|
|
||
| The deployment manifests include an `items` projection in the skills volume that maps ConfigMap keys to subdirectory paths. When adding skills, update both the ConfigMap and the volume spec's `items` entries to match. For multi-skill setups, consider using a PVC instead of a ConfigMap. | ||
|
|
Contributor
There was a problem hiding this comment.
Use backend-specific skills ConfigMap names in the skills command example.
Line 706 hardcodes claude-skills, but the vLLM manifest mounts claude-vllm-skills (deployment-vllm.yaml Line 83 and Line 210). In Option C/Option D, copy-pasting this creates an unused ConfigMap and skills won’t be picked up unless users also retarget mounts. Add the same “name varies by deployment option” note here (as you already did for MCP/settings), with the valid skills ConfigMap names per option.
Suggested doc fix
-oc create configmap claude-skills \
+oc create configmap <skills-configmap-name> \
--from-file=code-review-skill=./skills/code-review/SKILL.md \
--from-file=security-audit-skill=./skills/security-audit/SKILL.md
+
+# Use the skills ConfigMap name for your deployment option:
+# - Option A: claude-skills
+# - Option B: claude-vertex-skills
+# - Option C: claude-vllm-skills
+# - Option D: claude-ogx-vllm-skillsAs per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
📝 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.
Suggested change
| # Create a ConfigMap with skills | |
| oc create configmap claude-skills \ | |
| --from-file=code-review-skill=./skills/code-review/SKILL.md \ | |
| --from-file=security-audit-skill=./skills/security-audit/SKILL.md | |
| ``` | |
| The deployment manifests include an `items` projection in the skills volume that maps ConfigMap keys to subdirectory paths. When adding skills, update both the ConfigMap and the volume spec's `items` entries to match. For multi-skill setups, consider using a PVC instead of a ConfigMap. | |
| # Create a ConfigMap with skills | |
| oc create configmap <skills-configmap-name> \ | |
| --from-file=code-review-skill=./skills/code-review/SKILL.md \ | |
| --from-file=security-audit-skill=./skills/security-audit/SKILL.md | |
| # Use the skills ConfigMap name for your deployment option: | |
| # - Option A: claude-skills | |
| # - Option B: claude-vertex-skills | |
| # - Option C: claude-vllm-skills | |
| # - Option D: claude-ogx-vllm-skills | |
| The deployment manifests include an `items` projection in the skills volume that maps ConfigMap keys to subdirectory paths. When adding skills, update both the ConfigMap and the volume spec's `items` entries to match. For multi-skill setups, consider using a PVC instead of a ConfigMap. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agents/claude-code/deployment/README.md` around lines 705 - 712, The README
example hardcodes the ConfigMap name "claude-skills" which doesn't match
backend-specific mounts (for example the vLLM manifest mounts
"claude-vllm-skills" in deployment-vllm.yaml), so update the example command to
either use a placeholder/name variable or show the correct name per deployment
option and add a short note listing the valid skills ConfigMap names per option
(e.g., mention "claude-vllm-skills" for vLLM and the default name used by the
other deployment), and remind users to update the volume spec's items entries to
match the chosen ConfigMap when adding skills (affecting the skills volume items
projection).
Source: Coding guidelines
Contributor
Author
|
@andrewdonheiser Thanks for the review. Everything should be addressed in the latest commit. |
jwm4
approved these changes
Jun 8, 2026
jwm4
left a comment
jwm4
left a comment
Contributor
There was a problem hiding this comment.
I've been using this content, and it has been very helpful. I have a long list of follow-on items, but nothing that should be a blocker for this PR merging.
andrewdonheiser
self-requested a review
June 8, 2026 17:57
andrewdonheiser
approved these changes
Jun 8, 2026
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Jira Ticket
RHAIENG-4740
RHAIENG-4743
RHAIENG-4744
Testing
Related PRs
#92