feat(aws): add --tool-call-parser and --chat-template flags for RHEL AI#848
Open
are-ces wants to merge 1 commit into
Open
feat(aws): add --tool-call-parser and --chat-template flags for RHEL AI#848are-ces wants to merge 1 commit into
are-ces wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds optional RHEL AI tool-call parser, chat template, and max model length inputs across the API, CLI, provider setup, and Tekton wiring. Also adds Azure RHEL AI Tekton parameters for GPU count and GPU manufacturer, and forwards them into the create command. ChangesRHEL AI Tool-Call Parser and Chat Template
Azure RHEL AI GPU Parameters
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@pkg/provider/aws/action/rhel-ai/rhelai.go`:
- Around line 380-388: The install.conf update in rhelai.go only runs when
toolCallParser is set, so a standalone chatTemplate value is dropped even though
it was accepted earlier. Update the config-building logic around the rhelai.go
script assembly to either validate the chatTemplate/toolCallParser dependency up
front or apply chatTemplate independently in this block, using the existing
toolCallParser and chatTemplate symbols so both settings are handled correctly.
- Around line 381-387: The shell command built in rhelai.go is directly
embedding user-controlled values from r.toolCallParser and r.chatTemplate into
the sed script, which can break the command or allow injection. Update the logic
around toolArgs and the fmt.Sprintf that appends to script so these values are
safely escaped or quoted for single-quoted shell/sed usage before insertion. Use
the existing rhelai configuration-building path as the place to sanitize both
fields consistently before generating install.conf updates.
In `@tkn/infra-aws-rhel-ai.yaml`:
- Around line 326-331: The cmd-building logic in the Tekton script appends raw
params for tool-call-parser and chat-template into an eval string, which can
break quoting and enable shell injection. Update the command assembly to stop
concatenating these params into cmd; instead, build argv incrementally in the
same script section that handles params and execute the command directly without
eval, using the existing parameter checks around tool-call-parser and
chat-template as the place to refactor.
In `@tkn/template/infra-aws-rhel-ai.yaml`:
- Around line 326-331: The command-building logic in the Tekton template is
unsafe because `cmd` is later executed with `eval`, and
`params.tool-call-parser` / `params.chat-template` are being appended as raw
quoted text. Update the script to stop concatenating Tekton params into the eval
string; instead, build the argument list incrementally in the same section that
sets `cmd`, and execute the command directly without `eval`. Use the existing
parameter-handling block around `tool-call-parser` and `chat-template` as the
place to switch from string concatenation to argv-based execution.
🪄 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: febda5c2-da8c-430e-82cd-93979e54553a
📒 Files selected for processing (6)
cmd/mapt/cmd/aws/hosts/rhelai.gocmd/mapt/cmd/params/params.gopkg/provider/aws/action/rhel-ai/rhelai.gopkg/target/host/rhelai/api.gotkn/infra-aws-rhel-ai.yamltkn/template/infra-aws-rhel-ai.yaml
Comment on lines
+380
to
+388
| if len(*r.toolCallParser) > 0 { | ||
| toolArgs := fmt.Sprintf(`--enable-auto-tool-choice \\\n --tool-call-parser %s`, *r.toolCallParser) | ||
| if len(*r.chatTemplate) > 0 { | ||
| toolArgs += fmt.Sprintf(` \\\n --chat-template %s`, *r.chatTemplate) | ||
| } | ||
| script += fmt.Sprintf( | ||
| ` && sudo sed -i 's|--max-model-len.*|--max-model-len 4096 \\\n %s|' %s/install.conf`, | ||
| toolArgs, confDir) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
--chat-template is silently ignored unless --tool-call-parser is also set.
Line 380 gates the entire config update on toolCallParser, so a standalone --chat-template value is accepted by the CLI/Tekton layers but never reaches install.conf. Either validate that dependency earlier or handle chatTemplate independently here.
🤖 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 `@pkg/provider/aws/action/rhel-ai/rhelai.go` around lines 380 - 388, The
install.conf update in rhelai.go only runs when toolCallParser is set, so a
standalone chatTemplate value is dropped even though it was accepted earlier.
Update the config-building logic around the rhelai.go script assembly to either
validate the chatTemplate/toolCallParser dependency up front or apply
chatTemplate independently in this block, using the existing toolCallParser and
chatTemplate symbols so both settings are handled correctly.
Comment on lines
+381
to
+387
| toolArgs := fmt.Sprintf(`--enable-auto-tool-choice \\\n --tool-call-parser %s`, *r.toolCallParser) | ||
| if len(*r.chatTemplate) > 0 { | ||
| toolArgs += fmt.Sprintf(` \\\n --chat-template %s`, *r.chatTemplate) | ||
| } | ||
| script += fmt.Sprintf( | ||
| ` && sudo sed -i 's|--max-model-len.*|--max-model-len 4096 \\\n %s|' %s/install.conf`, | ||
| toolArgs, confDir) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Escape these values before embedding them into the sed command.
toolCallParser and chatTemplate are user-controlled inputs, but here they are inserted straight into a single-quoted shell command. A value containing ', |, &, or shell separators can corrupt install.conf or break out of the command during auto-start.
🤖 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 `@pkg/provider/aws/action/rhel-ai/rhelai.go` around lines 381 - 387, The shell
command built in rhelai.go is directly embedding user-controlled values from
r.toolCallParser and r.chatTemplate into the sed script, which can break the
command or allow injection. Update the logic around toolArgs and the fmt.Sprintf
that appends to script so these values are safely escaped or quoted for
single-quoted shell/sed usage before insertion. Use the existing rhelai
configuration-building path as the place to sanitize both fields consistently
before generating install.conf updates.
Comment on lines
+326
to
+331
| if [[ "$(params.tool-call-parser)" != "" ]]; then | ||
| cmd+="--tool-call-parser '$(params.tool-call-parser)' " | ||
| fi | ||
| if [[ "$(params.chat-template)" != "" ]]; then | ||
| cmd+="--chat-template '$(params.chat-template)' " | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not append raw Tekton params into the eval command string.
These values flow into cmd and are executed via eval later in the script. A tool-call-parser or chat-template containing a single quote can break quoting and run arbitrary shell in the task pod, exposing the mounted credentials. Build argv incrementally and exec it directly instead of extending the eval string.
🤖 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 `@tkn/infra-aws-rhel-ai.yaml` around lines 326 - 331, The cmd-building logic in
the Tekton script appends raw params for tool-call-parser and chat-template into
an eval string, which can break quoting and enable shell injection. Update the
command assembly to stop concatenating these params into cmd; instead, build
argv incrementally in the same script section that handles params and execute
the command directly without eval, using the existing parameter checks around
tool-call-parser and chat-template as the place to refactor.
Comment on lines
+326
to
+331
| if [[ "$(params.tool-call-parser)" != "" ]]; then | ||
| cmd+="--tool-call-parser '$(params.tool-call-parser)' " | ||
| fi | ||
| if [[ "$(params.chat-template)" != "" ]]; then | ||
| cmd+="--chat-template '$(params.chat-template)' " | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not append raw Tekton params into the eval command string.
These values flow into cmd and are executed via eval later in the script. A tool-call-parser or chat-template containing a single quote can break quoting and run arbitrary shell in the task pod, exposing the mounted credentials. Build argv incrementally and exec it directly instead of extending the eval string.
🤖 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 `@tkn/template/infra-aws-rhel-ai.yaml` around lines 326 - 331, The
command-building logic in the Tekton template is unsafe because `cmd` is later
executed with `eval`, and `params.tool-call-parser` / `params.chat-template` are
being appended as raw quoted text. Update the script to stop concatenating
Tekton params into the eval string; instead, build the argument list
incrementally in the same section that sets `cmd`, and execute the command
directly without `eval`. Use the existing parameter-handling block around
`tool-call-parser` and `chat-template` as the place to switch from string
concatenation to argv-based execution.
are-ces
force-pushed
the
rhelai-tool-calling
branch
2 times, most recently
from
98e4848 to
2d7165f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@cmd/mapt/cmd/params/params.go`:
- Around line 136-137: The standalone --chat-template flag is accepted in the
CLI but ignored unless --tool-call-parser is also set, so the dependency needs
to be enforced or the flag wiring fixed. Update the RHEL AI flow in
cmd/mapt/cmd/aws/hosts/rhelai.go and pkg/provider/aws/action/rhel-ai/rhelai.go
so that either chat-template cannot be used without tool-call-parser (fail fast
with validation) or the provider always passes through chat-template when set,
regardless of toolCallParser.
In `@tkn/infra-azure-rhel-ai.yaml`:
- Around line 88-90: The Tekton param default for gpus is forcing the Azure RHEL
AI task to always pass --gpus 8 when callers omit the value, bypassing the
provider’s unset/0 normalization path. Update the gpus parameter in
infra-azure-rhel-ai.yaml so it does not default to 8, and let the existing
CLI/provider handling in the task logic around the gpus condition at the
referenced block decide the effective GPU count unless a caller explicitly opts
in.
- Around line 238-243: The command-building logic in the Tekton task is
vulnerable because params like gpus and gpu-manufacturer are interpolated into
cmd and later executed with eval. Harden the command assembly by validating or
sanitizing these params before appending them, or avoid eval entirely by passing
arguments safely; update the logic in the script section where cmd is built so
single quotes or shell metacharacters cannot break out of the intended argument.
🪄 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: 4280c80e-9c50-42e3-ba62-d4cda787ff9a
📒 Files selected for processing (7)
cmd/mapt/cmd/aws/hosts/rhelai.gocmd/mapt/cmd/params/params.gopkg/provider/aws/action/rhel-ai/rhelai.gopkg/target/host/rhelai/api.gotkn/infra-aws-rhel-ai.yamltkn/infra-azure-rhel-ai.yamltkn/template/infra-aws-rhel-ai.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/target/host/rhelai/api.go
- cmd/mapt/cmd/aws/hosts/rhelai.go
- pkg/provider/aws/action/rhel-ai/rhelai.go
Comment on lines
+88
to
+90
| - name: gpus | ||
| description: Number of GPUs for the cloud instance (valid marketplace values are 1, 2, 4, 8) | ||
| default: "8" |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Preserve the Azure RHEL AI GPU default unless callers opt in.
With default: "8", the condition on Line 238 is always true for omitted Tekton params, so this task now always passes --gpus 8. That bypasses the CLI/provider default path where unset/0 GPUs are normalized to 1, which can unexpectedly select larger marketplace SKUs and hit quota/cost limits.
Proposed fix
- name: gpus
description: Number of GPUs for the cloud instance (valid marketplace values are 1, 2, 4, 8)
- default: "8"
+ default: ""As per path instructions, **: “Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.”
Also applies to: 238-240
🤖 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 `@tkn/infra-azure-rhel-ai.yaml` around lines 88 - 90, The Tekton param default
for gpus is forcing the Azure RHEL AI task to always pass --gpus 8 when callers
omit the value, bypassing the provider’s unset/0 normalization path. Update the
gpus parameter in infra-azure-rhel-ai.yaml so it does not default to 8, and let
the existing CLI/provider handling in the task logic around the gpus condition
at the referenced block decide the effective GPU count unless a caller
explicitly opts in.
Source: Path instructions
Comment on lines
+238
to
+243
| if [[ "$(params.gpus)" != "" ]]; then | ||
| cmd+="--gpus '$(params.gpus)' " | ||
| fi | ||
| if [[ "$(params.gpu-manufacturer)" != "" ]]; then | ||
| cmd+="--gpu-manufacturer '$(params.gpu-manufacturer)' " | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate new params before adding them to the eval’d command.
These Tekton params are interpolated into cmd and executed via eval on Line 272. A value containing a single quote can break out of the intended argument and execute shell syntax.
Proposed hardening
if [[ "$(params.gpus)" != "" ]]; then
- cmd+="--gpus '$(params.gpus)' "
+ case "$(params.gpus)" in
+ *[!0-9]*) echo "Parameter gpus must be numeric"; exit 1 ;;
+ esac
+ cmd+="--gpus $(params.gpus) "
fi
if [[ "$(params.gpu-manufacturer)" != "" ]]; then
+ case "$(params.gpu-manufacturer)" in
+ *[!A-Za-z0-9._-]*) echo "Parameter gpu-manufacturer contains unsupported characters"; exit 1 ;;
+ esac
cmd+="--gpu-manufacturer '$(params.gpu-manufacturer)' "
fiAs per path instructions, **: “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
| if [[ "$(params.gpus)" != "" ]]; then | |
| cmd+="--gpus '$(params.gpus)' " | |
| fi | |
| if [[ "$(params.gpu-manufacturer)" != "" ]]; then | |
| cmd+="--gpu-manufacturer '$(params.gpu-manufacturer)' " | |
| fi | |
| if [[ "$(params.gpus)" != "" ]]; then | |
| case "$(params.gpus)" in | |
| *[!0-9]*) echo "Parameter gpus must be numeric"; exit 1 ;; | |
| esac | |
| cmd+="--gpus $(params.gpus) " | |
| fi | |
| if [[ "$(params.gpu-manufacturer)" != "" ]]; then | |
| case "$(params.gpu-manufacturer)" in | |
| *[!A-Za-z0-9._-]*) echo "Parameter gpu-manufacturer contains unsupported characters"; exit 1 ;; | |
| esac | |
| cmd+="--gpu-manufacturer '$(params.gpu-manufacturer)' " | |
| fi |
🤖 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 `@tkn/infra-azure-rhel-ai.yaml` around lines 238 - 243, The command-building
logic in the Tekton task is vulnerable because params like gpus and
gpu-manufacturer are interpolated into cmd and later executed with eval. Harden
the command assembly by validating or sanitizing these params before appending
them, or avoid eval entirely by passing arguments safely; update the logic in
the script section where cmd is built so single quotes or shell metacharacters
cannot break out of the intended argument.
Source: Path instructions
Enable vLLM tool calling by adding --tool-call-parser and --chat-template flags. When --tool-call-parser is set, --enable-auto-tool-choice is automatically added. Chat template filename is resolved to /opt/app-root/template/. Updated Tekton task template with new params. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
are-ces
force-pushed
the
rhelai-tool-calling
branch
from
2d7165f to
872698e
Compare
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
1 participant
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.
Summary
Add
--tool-call-parser,--chat-template, and--max-model-lenflags tomapt aws rhel-ai createto enable vLLM tool calling support and configurable context window on RHEL AI instances.--tool-call-parser: enables tool calling with the specified parser. Automatically adds--enable-auto-tool-choiceto vLLM args.--chat-template: takes just the filename (e.g.tool_chat_template_llama3.1_json.jinja) — resolved to/opt/app-root/template/inside the RHAIIS container.--max-model-len: overrides the default 4096 token context window. Required for tool calling which adds significant token overhead.Usage:
mapt aws rhel-ai create \ --auto-start \ --model meta-llama/Llama-3.1-8B-Instruct \ --tool-call-parser llama3_json \ --chat-template tool_chat_template_llama3.1_json.jinja \ --max-model-len 16384 \ --expose-ports 8000 \ ...Available chat templates (inside RHAIIS container at
/opt/app-root/template/):tool_chat_template_llama3.1_json.jinjatool_chat_template_llama3.2_json.jinjatool_chat_template_llama4_json.jinjatool_chat_template_mistral.jinjatool_chat_template_hermes.jinjatool_chat_template_granite.jinjaUpdated Tekton task template with all new params.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com