Repository navigation
feat(release): bump v1.2.0 with version sync and engine docker auto-triggers - #695
Conversation
…ine docker workflows Version bumps (v1.1.x → v1.2.0): - model_gateway/Cargo.toml: 1.1.0 → 1.2.0 - bindings/python/Cargo.toml: 1.1.0 → 1.2.0 - bindings/golang/Cargo.toml: 1.1.0 → 1.2.0 - bindings/python/pyproject.toml: 1.1.0 → 1.2.0 - All workspace crates bumped per conventional commit analysis - Workspace root Cargo.toml dependency versions synced check_release_versions.sh improvements: - Add SMG_VERSION_SYNC registry: bindings/python/Cargo.toml, bindings/golang/Cargo.toml, bindings/python/pyproject.toml, and 4 workflow files must mirror model_gateway version - Add smg-client and openapi-gen to CRATES array - Add Phase 1c to detect version sync mismatches - Add Phase 2 display and Phase 3 apply logic for sync entries - Add get_workflow_smg_version/set_workflow_smg_version helpers Engine docker workflow changes: - Add push trigger on main (paths: bindings/python/pyproject.toml) to release-sglang-docker, release-vllm-docker, release-trtllm-docker so they auto-trigger on version bumps like release-pypi.yml - Add fallback defaults (|| 'value') in job with: blocks for vllm and trtllm so push events use sensible defaults - Add default base_image_ref for trtllm: nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6 - Update smg_commit defaults from v1.1.0 to v1.2.0 across all engine workflows and _build-engine-image.yml Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
github-actions
Bot
added
python-bindings
📝 WalkthroughWalkthroughThis PR performs a coordinated version bump across multiple workspace crates and bindings to v1.2.0, updates Docker release workflow defaults and triggers to reference the new version, and extends the release verification script with SMG version synchronization logic to ensure consistency across Cargo manifests, Python package metadata, and workflow files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 710c93b37e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| with: | ||
| engine: trtllm | ||
| base_image_ref: ${{ inputs.base_image_ref }} | ||
| base_image_ref: ${{ inputs.base_image_ref || 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6' }} |
There was a problem hiding this comment.
Using inputs.base_image_ref || 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6' makes it impossible to pass an empty base_image_ref from workflow_dispatch, even though this workflow documents “Empty = build from source.” In this repo, _build-engine-image.yml only takes the source-build path when inputs.base_image_ref == '', so manual runs that are supposed to build TRT-LLM from source will now always use the default base image and skip source builds.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/_build-engine-image.yml:
- Around line 32-35: Change the smg_commit input default from 'v1.2.0' to
'latest' so auto-triggered runs on push use HEAD instead of a
potentially-nonexistent release tag; update the input declaration for smg_commit
in this reusable workflow (the "smg_commit" input block) to default: 'latest',
and also mirror this change or ensure explicit tag passing in the caller
workflows (release-vllm-docker.yml, release-trtllm-docker.yml,
release-sglang-docker.yml) so release-tagged runs can still supply a fixed tag.
In @.github/workflows/release-trtllm-docker.yml:
- Around line 18-20: The workflow input base_image_ref currently has a default
which makes an empty manual input falsy and prevents "build from source"; change
the logic so the default value is used only for push events and manual runs
accept an empty string. Specifically, remove the unconditional default behavior
for the workflow input base_image_ref and in the places where it's consumed
(referencing the input name base_image_ref) use a conditional expression that
uses the NVCR default only when github.event_name == 'push' (e.g.
github.event_name == 'push' ? 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6' :
inputs.base_image_ref) and apply the same change for the second occurrence of
base_image_ref to ensure manual workflow_dispatch runs can pass an empty value
to trigger building from source.
In `@scripts/check_release_versions.sh`:
- Around line 345-364: get_workflow_smg_version and set_workflow_smg_version
treat the workflow's SMG token as a single scalar version, but workflows can
contain multiple different SMG occurrences (smg_commit.default,
inputs.smg_commit || '...', run-name fallbacks, etc.); update the logic so you
parse and collect all distinct "vX.Y.Z" occurrences from the file (e.g., via
grep/regex) in get_workflow_smg_version and return a set/list, and change
set_workflow_smg_version to replace every unique matching token (use
escape_version and sed_inplace helpers) by the new version and then verify that
no old-version tokens remain (grep for any "v" followed by escaped_old); ensure
verification fails if any stale SMG tokens remain so mixed-version files are
fixed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f3242ae4-47fa-40e5-9b52-8f93c5d8c14a
📒 Files selected for processing (22)
.github/workflows/_build-engine-image.yml.github/workflows/release-sglang-docker.yml.github/workflows/release-trtllm-docker.yml.github/workflows/release-vllm-docker.ymlCargo.tomlbindings/golang/Cargo.tomlbindings/python/Cargo.tomlbindings/python/pyproject.tomlcrates/data_connector/Cargo.tomlcrates/grpc_client/Cargo.tomlcrates/kv_index/Cargo.tomlcrates/mcp/Cargo.tomlcrates/mesh/Cargo.tomlcrates/multimodal/Cargo.tomlcrates/protocols/Cargo.tomlcrates/reasoning_parser/Cargo.tomlcrates/tokenizer/Cargo.tomlcrates/tool_parser/Cargo.tomlcrates/wasm/Cargo.tomlcrates/workflow/Cargo.tomlmodel_gateway/Cargo.tomlscripts/check_release_versions.sh
| smg_commit: | ||
| description: 'SMG commit/ref ("latest" for HEAD)' | ||
| required: false | ||
| default: 'v1.1.0' | ||
| default: 'v1.2.0' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Call sites for _build-engine-image and smg_commit =="
rg -n -C3 'uses:\s*\.\/\.github\/workflows\/_build-engine-image\.yml|smg_commit:' .github/workflows
echo
echo "== SMG_COMMIT handling in docker/engine.Dockerfile =="
if [ -f docker/engine.Dockerfile ]; then
rg -n -C3 'ARG\s+SMG_COMMIT|SMG_COMMIT|git\s+(clone|checkout|fetch)' docker/engine.Dockerfile
else
echo "docker/engine.Dockerfile not found"
fiRepository: lightseekorg/smg
Length of output: 278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Call sites for _build-engine-image reusable workflow =="
rg -n '_build-engine-image' .github/workflows
echo
echo "== smg_commit input usage in callers =="
rg -n -C5 'smg_commit' .github/workflows
echo
echo "== SMG_COMMIT handling in docker/engine.Dockerfile =="
if [ -f docker/engine.Dockerfile ]; then
rg -n 'SMG_COMMIT\|ARG SMG_COMMIT' docker/engine.Dockerfile
else
echo "docker/engine.Dockerfile not found"
fiRepository: lightseekorg/smg
Length of output: 12024
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Checking for docker/engine.Dockerfile =="
find . -name "engine.Dockerfile" -o -name "*engine*.Dockerfile" | head -20
echo
echo "== All Dockerfiles in docker/ directory =="
ls -la docker/ 2>/dev/null | grep -i dockerfile || echo "docker/ directory or Dockerfiles not found"
echo
echo "== Search for SMG_COMMIT in all Dockerfiles =="
find . -name "*.Dockerfile" -o -name "Dockerfile*" | xargs grep -l "SMG_COMMIT" 2>/dev/null || echo "SMG_COMMIT not found in any Dockerfile"Repository: lightseekorg/smg
Length of output: 378
🏁 Script executed:
cat docker/engine.DockerfileRepository: lightseekorg/smg
Length of output: 2588
Don't pin smg_commit default to a release tag when auto-triggered on push to main.
Line 35 defaults to v1.2.0, and the caller workflows (release-vllm-docker.yml, release-trtllm-docker.yml, release-sglang-docker.yml) that invoke this reusable workflow also default to v1.2.0. When these workflows auto-trigger on push to main, the tag may not exist yet, causing docker/engine.Dockerfile to fail at git checkout v1.2.0. Use latest as the default to allow the Dockerfile to clone HEAD instead, and let release-tag callers pass the tag explicitly.
Suggested fix
- default: 'v1.2.0'
+ default: 'latest'Note: The same timing risk exists in the caller workflows themselves (release-vllm-docker.yml, release-trtllm-docker.yml, release-sglang-docker.yml), which also default smg_commit to v1.2.0.
📝 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.
| smg_commit: | |
| description: 'SMG commit/ref ("latest" for HEAD)' | |
| required: false | |
| default: 'v1.1.0' | |
| default: 'v1.2.0' | |
| smg_commit: | |
| description: 'SMG commit/ref ("latest" for HEAD)' | |
| required: false | |
| default: 'latest' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/_build-engine-image.yml around lines 32 - 35, Change the
smg_commit input default from 'v1.2.0' to 'latest' so auto-triggered runs on
push use HEAD instead of a potentially-nonexistent release tag; update the input
declaration for smg_commit in this reusable workflow (the "smg_commit" input
block) to default: 'latest', and also mirror this change or ensure explicit tag
passing in the caller workflows (release-vllm-docker.yml,
release-trtllm-docker.yml, release-sglang-docker.yml) so release-tagged runs can
still supply a fixed tag.
| description: 'Base image (e.g. nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6). Empty = build from source.' | ||
| default: 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6' | ||
| required: false |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In GitHub Actions, if a workflow_dispatch string input is intentionally left empty and the workflow passes ${{ inputs.base_image_ref || 'fallback' }}, does the expression resolve to fallback? What pattern preserves an empty manual input while still defaulting only for push-triggered runs?
💡 Result:
Yes. In GitHub Actions expressions, an empty string ("") is falsy, so ${{ inputs.base_image_ref || 'fallback' }} will resolve to 'fallback' when the manual workflow_dispatch input is intentionally cleared. [1]
To preserve an intentionally empty manual input, but still default for non-manual runs (e.g., push, where inputs are null/absent), use case():
base_image_ref: ${{ case(github.event_name == 'workflow_dispatch', inputs.base_image_ref, 'fallback') }}case() returns the value for the first true predicate without treating "" as a reason to fall back, and only uses 'fallback' for non-workflow_dispatch events. [1], [2]
Keep the "build from source" path reachable on manual runs.
An empty workflow_dispatch input for base_image_ref is falsy and will trigger the fallback, preventing users from actually building from source as documented on line 18. Default only for push events, and pass manual inputs through unchanged.
Suggested fix
- base_image_ref: ${{ inputs.base_image_ref || 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6' }}
+ base_image_ref: ${{ case(github.event_name == 'workflow_dispatch', inputs.base_image_ref, 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6') }}Also applies to: 51-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-trtllm-docker.yml around lines 18 - 20, The
workflow input base_image_ref currently has a default which makes an empty
manual input falsy and prevents "build from source"; change the logic so the
default value is used only for push events and manual runs accept an empty
string. Specifically, remove the unconditional default behavior for the workflow
input base_image_ref and in the places where it's consumed (referencing the
input name base_image_ref) use a conditional expression that uses the NVCR
default only when github.event_name == 'push' (e.g. github.event_name == 'push'
? 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6' : inputs.base_image_ref) and
apply the same change for the second occurrence of base_image_ref to ensure
manual workflow_dispatch runs can pass an empty value to trigger building from
source.
| # Extract smg_commit default version from a workflow file (e.g. "default: 'v1.2.0'") | ||
| get_workflow_smg_version() { | ||
| local file="$1" | ||
| grep -m1 "default: 'v[0-9]" "$file" | sed "s/.*default: 'v\([^']*\)'.*/\1/" | ||
| } | ||
|
|
||
| # Update all smg version references in a workflow file. | ||
| # Replaces the version in default: values, || fallbacks, and description examples. | ||
| set_workflow_smg_version() { | ||
| local file="$1" | ||
| local old_version="$2" | ||
| local new_version="$3" | ||
| local escaped_old | ||
| escaped_old=$(escape_version "$old_version") | ||
| sed_inplace "s/v${escaped_old}/v${new_version}/g" "$file" | ||
| if ! grep -q "default: 'v${new_version}'" "$file"; then | ||
| echo -e " ${RED}FAILED to update $file${NC}" >&2 | ||
| return 1 | ||
| fi | ||
| } |
There was a problem hiding this comment.
Treat workflow SMG versions as a set, not a single scalar.
get_workflow_smg_version() and set_workflow_smg_version() assume every SMG reference inside a workflow already shares one old_version. That is no longer true in these files: the SMG ref now appears in smg_commit.default, in inputs.smg_commit || '...' fallbacks, and in release-sglang-docker.yml also in the run-name fallback. A mixed file like default=v1.2.0 + fallback v1.1.0 will still pass Phase 1c, and the fixer will not rewrite the stale token.
Also applies to: 522-543
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check_release_versions.sh` around lines 345 - 364,
get_workflow_smg_version and set_workflow_smg_version treat the workflow's SMG
token as a single scalar version, but workflows can contain multiple different
SMG occurrences (smg_commit.default, inputs.smg_commit || '...', run-name
fallbacks, etc.); update the logic so you parse and collect all distinct
"vX.Y.Z" occurrences from the file (e.g., via grep/regex) in
get_workflow_smg_version and return a set/list, and change
set_workflow_smg_version to replace every unique matching token (use
escape_version and sed_inplace helpers) by the new version and then verify that
no old-version tokens remain (grep for any "v" followed by escaped_old); ensure
verification fails if any stale SMG tokens remain so mixed-version files are
fixed.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's release management and continuous integration capabilities. It ensures version consistency across core components and their bindings by introducing a robust synchronization mechanism. Furthermore, it automates the triggering of engine-specific Docker image builds upon version updates, streamlining the release process and reducing manual intervention for critical deployment artifacts. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates numerous crate and package versions across the repository and introduces a new version synchronization mechanism in scripts/check_release_versions.sh. This mechanism aims to automate the process of ensuring that versions in various project files (Cargo.toml, pyproject.toml, and workflow files) are consistent with the main smg (model_gateway) version. However, the newly added set_workflow_smg_version function has a high-severity command injection vulnerability due to unsafe sed usage with unescaped variables, and could also corrupt files if old_version is empty. Additionally, the get_workflow_smg_version function uses an overly broad grep pattern, risking incorrect version extraction from comments, which could lead to dangerous version substitutions.
| local escaped_old | ||
| escaped_old=$(escape_version "$old_version") | ||
| sed_inplace "s/v${escaped_old}/v${new_version}/g" "$file" |
There was a problem hiding this comment.
The set_workflow_smg_version function is vulnerable to command injection via sed due to the unsafe use of unescaped variables (escaped_old, new_version) sourced from repository files. This could allow an attacker to inject sed commands and achieve remote code execution. For example, a malicious workflow file could lead to arbitrary command execution. Furthermore, if old_version is empty, the sed command on line 359 could corrupt the workflow file. To remediate, properly escape sed delimiters in all variables or use safer alternatives like awk, and add a guard to handle empty old_version gracefully.
| local escaped_old | |
| escaped_old=$(escape_version "$old_version") | |
| sed_inplace "s/v${escaped_old}/v${new_version}/g" "$file" | |
| local escaped_old | |
| escaped_old=$(escape_version "$old_version") | |
| local escaped_new | |
| escaped_new=$(echo "$new_version" | sed 's/\//\\\//g') | |
| local escaped_old_sed | |
| escaped_old_sed=$(echo "$escaped_old" | sed 's/\//\\\//g') | |
| sed_inplace "s/v${escaped_old_sed}/v${escaped_new}/g" "$file" |
| # Extract smg_commit default version from a workflow file (e.g. "default: 'v1.2.0'") | ||
| get_workflow_smg_version() { | ||
| local file="$1" | ||
| grep -m1 "default: 'v[0-9]" "$file" | sed "s/.*default: 'v\([^']*\)'.*/\1/" |
There was a problem hiding this comment.
The grep pattern default: 'v[0-9] is too broad. It could incorrectly match version strings inside comments or descriptions, leading to partial or incorrect version extraction. For example, if a file contains description: "uses default: 'v1' as example", this function would extract 1. This incorrect version would then be passed to set_workflow_smg_version, potentially causing a dangerous substitution like s/v1/v1.2.0/g.
To prevent this, the pattern should be anchored to the beginning of the line to ensure it only matches actual default: keys in the YAML structure.
| grep -m1 "default: 'v[0-9]" "$file" | sed "s/.*default: 'v\([^']*\)'.*/\1/" | |
| grep -m1 "^[[:space:]]*default: 'v[0-9]" "$file" | sed "s/.*default: 'v\([^']\|\)\'.*/\1/" |
1 participant
Summary
Bumps all workspace crates to v1.2.0 based on conventional commit analysis, and improves the release infrastructure so engine docker workflows (sglang, vllm, trtllm) auto-trigger on version bumps — matching the existing behavior of
release-pypi.ymlandrelease-docker.yml.What changed
Version bumps (22 files)
model_gateway/Cargo.toml: 1.1.0 → 1.2.0bindings/python/Cargo.toml: 1.1.0 → 1.2.0bindings/golang/Cargo.toml: 1.1.0 → 1.2.0bindings/python/pyproject.toml: 1.1.0 → 1.2.0Cargo.tomlworkspace dependency versions syncedscripts/check_release_versions.shbindings/python/Cargo.toml,bindings/golang/Cargo.toml,bindings/python/pyproject.toml, and 4 workflow files in sync withmodel_gateway/Cargo.tomlversionsmg-clientandopenapi-gento the CRATES arrayget_workflow_smg_version()/set_workflow_smg_version()for reading/updatingdefault: 'vX.Y.Z'and|| 'vX.Y.Z'patterns in workflow filesEngine docker workflows
pushtrigger onmainwatchingbindings/python/pyproject.tomltorelease-sglang-docker.yml,release-vllm-docker.yml,release-trtllm-docker.yml— they now auto-release on version bumps likerelease-pypi.yml|| 'value'fallbacks in jobwith:blocks for vllm and trtllm sopushevents use sensible defaultsdefault: 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc6'forbase_image_refsmg_commitdefaults fromv1.1.0→v1.2.0across all engine workflows and_build-engine-image.ymlWhy
bindings/python/pyproject.toml,bindings/python/Cargo.toml, andbindings/golang/Cargo.tomlwere never updated by the version check script, causing them to drift from the gateway versionv1.1.0and nopushtrigger, so they would never auto-run on releasebase_image_ref, requiring manual input every timeTest plan
make check-versionsend-to-end — all versions detected correctly, sync mismatches found and fixed||fallbacks matchdefault:values in all engine workflowsSummary by CodeRabbit