Repository navigation
ci(docker): add TokenSpeed engine image release workflow - #1927
Conversation
Add an SMG+TokenSpeed docker image release mirroring the existing
sglang/vllm/trtllm engine workflows. TokenSpeed publishes a self-contained
engine image (lightseekorg/tokenspeed:tml), so this uses the thin-wrapper
base-image path rather than a source build.
- release-tokenspeed-docker.yml: thin wrapper over _build-engine-image
- _build-engine-image.yml / engine.Dockerfile: allow engine=tokenspeed
- engine.Dockerfile: drop tokenspeed-smg-grpc-{proto,servicer} before
install-smg.sh so SMG's own gRPC modules win (avoids stale descriptors)
- install-tokenspeed.sh: editable install for the optional source path
- nightly-engine-docker.yml: add tokenspeed to the nightly matrix
Signed-off-by: key4ng <rukeyang@gmail.com>
📝 WalkthroughWalkthroughTokenSpeed is added as a supported engine for reusable and nightly builds. A dedicated release workflow is introduced, and Docker image installation removes prebuilt SMG gRPC packages before installing TokenSpeed from local source. ChangesTokenSpeed Docker image support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant ReusableBuildWorkflow
participant EngineDockerfile
ReleaseWorkflow->>ReusableBuildWorkflow: pass TokenSpeed build parameters
ReusableBuildWorkflow->>EngineDockerfile: build with ENGINE=tokenspeed
EngineDockerfile->>EngineDockerfile: remove preinstalled SMG gRPC packages
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request adds support for the tokenspeed engine in the Docker engine build, handling potential import shadowing by uninstalling pre-baked tokenspeed-smg-grpc-proto and tokenspeed-smg-grpc-servicer packages, and introducing a script to install tokenspeed from source. The reviewer suggests avoiding the --editable flag in the pip install command within the Docker build script, as it creates a runtime dependency on the source directory.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| TS_SRC="${1:-/tmp/tokenspeed-src}" | ||
| cd "${TS_SRC}/python" | ||
| pip install --no-deps --force-reinstall --editable . |
There was a problem hiding this comment.
Using --editable (or -e) in a Docker image build is generally discouraged for production-ready images. It creates a direct dependency on the source directory remaining present at the exact same path at runtime. If the source directory is deleted or modified to optimize image size, the installation will break. Consider performing a standard install instead.
| pip install --no-deps --force-reinstall --editable . | |
| pip install --no-deps --force-reinstall . |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19b5c28d11
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "${ENGINE}" in | ||
| vllm|sglang|trtllm|tgl) ;; | ||
| *) echo "ERROR: Unknown engine '${ENGINE}'. Must be one of: vllm, sglang, trtllm, tgl." >&2; exit 1 ;; | ||
| vllm|sglang|trtllm|tgl|tokenspeed) ;; |
There was a problem hiding this comment.
Add TokenSpeed serve support before enabling images
Accepting tokenspeed here lets the new release/nightly callers build images with ENGINE=tokenspeed; with no backend override, docker/engine.Dockerfile:53 exports SMG_DEFAULT_BACKEND=tokenspeed, but smg serve still builds its choices only from BACKEND_ARG_ADDERS (sglang, vllm, trtllm) in bindings/python/src/smg/serve.py:438-459. As a result the produced TokenSpeed all-in-one image fails argument parsing before it can start a worker, and there is no valid --backend tokenspeed path; please add a TokenSpeed launcher/parser entry or avoid setting an unsupported default for these images.
Useful? React with 👍 / 👎.
| smg_commit: | ||
| description: 'SMG commit/ref ("latest" for HEAD)' | ||
| required: false | ||
| default: 'v1.7.0' |
There was a problem hiding this comment.
Register TokenSpeed workflow in release version sync
This new workflow has the same smg_commit default as the other engine release workflows, but scripts/check_release_versions.sh:96-104 only tracks sglang/vllm/trtllm in SMG_VERSION_SYNC, and the release check/update loop uses only that list to validate and rewrite workflow defaults. On the next SMG version bump, make check-release-versions will not flag or update this TokenSpeed default, so manual TokenSpeed releases will keep building v1.7.0 unless the operator remembers to override the input.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.github/workflows/release-tokenspeed-docker.yml:
- Line 74: Replace secrets: inherit in the release workflow’s reusable-workflow
invocation with an explicit mapping of only the required secret(s), such as
GH_SYNC_TOKEN if used. Declare each passed secret in the called workflow’s
workflow_call configuration and remove reliance on inherited secrets.
- Line 7: Update the workflow’s smg assignment to default to the triggering
event’s SHA/ref instead of hardcoded main or v1.7.0, while preserving the manual
inputs.smg_commit override. For pull requests from forks, also pass the head
repository and head ref through the reusable workflow inputs so checkout targets
the PR’s actual revision.
- Around line 12-15: Broaden the push path filters in the release workflow’s
push trigger to include the TokenSpeed Dockerfile, installation script, and
reusable workflow paths alongside bindings/python/pyproject.toml, so relevant
changes merged to main publish a new image.
- Around line 54-56: Move packages: write from the workflow-level permissions
block into the jobs.build permissions block, while retaining contents: read at
the appropriate scope. Ensure only build receives registry-write access and
other jobs do not inherit it.
In `@docker/engine.Dockerfile`:
- Around line 69-78: Remove the “|| true” fallback from the conditional pip
uninstall command in the tokenspeed branch, allowing genuine uninstall failures
to stop the build while preserving successful behavior when packages are absent.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: cb60bbdb-63be-43d2-87ca-bf059d7b647d
📒 Files selected for processing (5)
.github/workflows/_build-engine-image.yml.github/workflows/nightly-engine-docker.yml.github/workflows/release-tokenspeed-docker.ymldocker/engine.Dockerfilescripts/installation/install-tokenspeed.sh
| SMG+TokenSpeed | | ||
| ${{ github.event_name == 'push' && 'base=lightseekorg/tokenspeed:tml' || format('base={0}', inputs.base_image_ref || 'lightseekorg/tokenspeed:tml') }} | | ||
| engine=${{ inputs.tokenspeed_commit || 'latest' }} | | ||
| smg=${{ github.event_name == 'pull_request' && 'main' || inputs.smg_commit || 'v1.7.0' }} | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Build the triggering revision, not main or v1.7.0. The reusable workflow currently checks out main for PR dry runs and v1.7.0 for pushes, so automatic runs never validate or publish the revision that triggered them. Pass the event SHA/ref through smg_commit and keep the manual input override; for fork PRs, pass the head repository/ref pair as well.
🤖 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 @.github/workflows/release-tokenspeed-docker.yml at line 7, Update the
workflow’s smg assignment to default to the triggering event’s SHA/ref instead
of hardcoded main or v1.7.0, while preserving the manual inputs.smg_commit
override. For pull requests from forks, also pass the head repository and head
ref through the reusable workflow inputs so checkout targets the PR’s actual
revision.
| push: | ||
| branches: [main] | ||
| paths: | ||
| - bindings/python/pyproject.toml |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workflow file ==\n'
wc -l .github/workflows/release-tokenspeed-docker.yml
cat -n .github/workflows/release-tokenspeed-docker.yml
printf '\n== search related references ==\n'
rg -n "release-tokenspeed-docker|pyproject.toml|docker|publish|dry run|pull_request|push:" .github/workflows bindings -SRepository: lightseekorg/smg
Length of output: 26489
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' .github/workflows/release-tokenspeed-docker.yml | cat -nRepository: lightseekorg/smg
Length of output: 3288
Broaden the push trigger for TokenSpeed releases. push only watches bindings/python/pyproject.toml, so Dockerfile, installation-script, or reusable-workflow changes merged to main won’t publish a new image unless the version file also changes. Add the release-relevant paths here, or require a version bump before merge.
🤖 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 @.github/workflows/release-tokenspeed-docker.yml around lines 12 - 15,
Broaden the push path filters in the release workflow’s push trigger to include
the TokenSpeed Dockerfile, installation script, and reusable workflow paths
alongside bindings/python/pyproject.toml, so relevant changes merged to main
publish a new image.
| permissions: | ||
| contents: read | ||
| packages: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/release-tokenspeed-docker.yml"
echo "== file outline =="
ast-grep outline "$file" --view expanded || true
echo
echo "== numbered file excerpt =="
cat -n "$file"Repository: lightseekorg/smg
Length of output: 3398
Scope packages: write to jobs.build. Workflow-level permissions apply to the whole workflow, so any later job would inherit registry-write access.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 56-56: overly broad permissions (excessive-permissions): packages: write is overly broad at the workflow level
(excessive-permissions)
[warning] 56-56: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🤖 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 @.github/workflows/release-tokenspeed-docker.yml around lines 54 - 56, Move
packages: write from the workflow-level permissions block into the jobs.build
permissions block, while retaining contents: read at the appropriate scope.
Ensure only build receives registry-write access and other jobs do not inherit
it.
Source: Linters/SAST tools
| smg_commit: ${{ github.event_name == 'pull_request' && 'main' || inputs.smg_commit || 'v1.7.0' }} | ||
| tag: ${{ inputs.tag }} | ||
| dry_run: ${{ github.event_name == 'pull_request' }} | ||
| secrets: inherit |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Avoid inheriting all caller secrets.
Pass only the named secret(s) required by the reusable workflow, such as GH_SYNC_TOKEN if needed, and declare them explicitly in the called workflow. secrets: inherit unnecessarily broadens the secret surface of this release job.
🤖 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 @.github/workflows/release-tokenspeed-docker.yml at line 74, Replace secrets:
inherit in the release workflow’s reusable-workflow invocation with an explicit
mapping of only the required secret(s), such as GH_SYNC_TOKEN if used. Declare
each passed secret in the called workflow’s workflow_call configuration and
remove reliance on inherited secrets.
Source: Linters/SAST tools
| # TokenSpeed base images bake in `tokenspeed-smg-grpc-proto` / | ||
| # `tokenspeed-smg-grpc-servicer`, which install the same `smg_grpc_proto` / | ||
| # `smg_grpc_servicer` import paths that install-smg.sh reinstalls from source. | ||
| # Left in place they can shadow the source installs and serve stale proto | ||
| # descriptors ("Method not found!"). Drop them first so SMG's own gRPC modules | ||
| # win; the install-smg.sh source installs below replace them. Scoped to | ||
| # tokenspeed so the other engine bases keep their exact pip behavior. | ||
| RUN if [ "${ENGINE}" = "tokenspeed" ]; then \ | ||
| pip uninstall -y tokenspeed-smg-grpc-proto tokenspeed-smg-grpc-servicer || true; \ | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section with line numbers
sed -n '1,140p' docker/engine.Dockerfile | nl -ba | sed -n '55,95p'
# Find where the tokenspeed packages are installed elsewhere in the repo
rg -n "tokenspeed-smg-grpc-(proto|servicer)|smg_grpc_(proto|servicer)|install-smg\.sh|ENGINE\"? *= *\"tokenspeed\"" -S .Repository: lightseekorg/smg
Length of output: 194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import subprocess, sys
pkg = "definitely-not-installed-pkg-xyz"
proc = subprocess.run(
[sys.executable, "-m", "pip", "uninstall", "-y", pkg],
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
)
print(proc.stdout)
print(f"exit_code={proc.returncode}")
PYRepository: lightseekorg/smg
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant Dockerfile slice with line numbers using awk (nl is unavailable here).
awk 'NR>=55 && NR<=95 { printf "%4d %s\n", NR, $0 }' docker/engine.Dockerfile
# Show the exact install-smg references and nearby context.
rg -n -C 3 "install-smg\.sh|tokenspeed-smg-grpc-(proto|servicer)|smg_grpc_(proto|servicer)|ENGINE" docker/engine.DockerfileRepository: lightseekorg/smg
Length of output: 5754
Remove the || true fallback. pip uninstall already exits 0 when a package is absent, so this only hides real removal failures and can leave stale smg_grpc_* modules in the image.
🤖 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 `@docker/engine.Dockerfile` around lines 69 - 78, Remove the “|| true” fallback
from the conditional pip uninstall command in the tokenspeed branch, allowing
genuine uninstall failures to stop the build while preserving successful
behavior when packages are absent.
… base The tokenspeed base image uses a debian-managed system Python. install-smg.sh's `pip install --upgrade pip` then fails with "Cannot uninstall pip 24.0, RECORD file not found", short-circuiting the maturin install and dying at `maturin: command not found` (exit 127). Shadow the debian-owned pip and pyyaml with pip-managed copies (--ignore-installed) in the tokenspeed-scoped prep step, mirroring the existing trtllm pyyaml special-case, so install-smg.sh can upgrade them normally. Signed-off-by: key4ng <rukeyang@gmail.com>
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 `@docker/engine.Dockerfile`:
- Line 83: Update the pip install command in the Dockerfile to pin both pip and
pyyaml to validated exact versions, or reference a constraints file that fixes
both versions. Preserve the existing no-cache and ignore-installed behavior
while ensuring TokenSpeed bootstrap dependencies are deterministic.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f920fe6a-a4a5-4e67-8aa9-812fa6ba3954
📒 Files selected for processing (1)
docker/engine.Dockerfile
| # found!"). Drop them first so SMG's own gRPC modules win. | ||
| # Scoped to tokenspeed so the other engine bases keep their exact pip behavior. | ||
| RUN if [ "${ENGINE}" = "tokenspeed" ]; then \ | ||
| pip install --no-cache-dir --ignore-installed pip pyyaml; \ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Pin the TokenSpeed bootstrap dependencies.
Line 83 resolves pip and pyyaml to whatever versions are latest at build time, making release images nondeterministic and allowing future incompatible releases to break install-smg.sh. Use validated exact versions or a constraints file for both packages.
🤖 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 `@docker/engine.Dockerfile` at line 83, Update the pip install command in the
Dockerfile to pin both pip and pyyaml to validated exact versions, or reference
a constraints file that fixes both versions. Preserve the existing no-cache and
ignore-installed behavior while ensuring TokenSpeed bootstrap dependencies are
deterministic.
Source: Linters/SAST tools
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48736e9c0e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # found!"). Drop them first so SMG's own gRPC modules win. | ||
| # Scoped to tokenspeed so the other engine bases keep their exact pip behavior. | ||
| RUN if [ "${ENGINE}" = "tokenspeed" ]; then \ | ||
| pip install --no-cache-dir --ignore-installed pip pyyaml; \ |
There was a problem hiding this comment.
Set break-system-packages before TokenSpeed pip installs
When building on the Debian system-Python TokenSpeed base described here, this first pip install still runs without PIP_BREAK_SYSTEM_PACKAGES=1 or --break-system-packages; TokenSpeed's own Dockerfile sets that env before installing packages (source). On those bases pip rejects system-environment installs before install-smg.sh runs, so the new TokenSpeed release/nightly image builds can fail before producing an image.
Useful? React with 👍 / 👎.
|
|
||
| TS_SRC="${1:-/tmp/tokenspeed-src}" | ||
| cd "${TS_SRC}/python" | ||
| pip install --no-deps --force-reinstall --editable . |
There was a problem hiding this comment.
Install all TokenSpeed source packages
When a caller supplies tokenspeed_repo to use the advertised source-install path, this only reinstalls the top-level ./python package; the upstream install flow installs the kernel and scheduler packages as well before verification (docs). That means source-built images can keep missing or stale tokenspeed-kernel/tokenspeed-scheduler code from the base image even though the workflow labels the image with the selected TokenSpeed commit.
Useful? React with 👍 / 👎.
Description
Problem
SMG publishes co-located gateway+engine Docker images for vLLM, SGLang, and TensorRT-LLM (
release-{vllm,sglang,trtllm}-docker.yml), but there is no equivalent release for TokenSpeed, even though TokenSpeed is a first-class supported backend (RuntimeType::TokenSpeed,tokenspeed_*gRPC clients, README/docs).Solution
TokenSpeed (
lightseekorg/tokenspeed) publishes a self-contained engine image (its docs point atlightseekorg/tokenspeed:tml; the nightly workflow also pushesnightly-<sha>), builtFROM lightseekorg/tokenspeed-runner:*with the kernel + scheduler + engine baked in. Because the engine is prebuilt, this uses the thin-wrapper base-image path (likerelease-sglang-docker.yml) rather than the trtllm source-build path.One TokenSpeed-specific quirk: its images bake in
tokenspeed-smg-grpc-proto/tokenspeed-smg-grpc-servicer, which claim the samesmg_grpc_proto/smg_grpc_servicerimport paths thatinstall-smg.shreinstalls from source. Left in place they can shadow the source installs and serve stale proto descriptors, so the Dockerfile uninstalls them first (scoped totokenspeed, mirroring the existing trtllmpyyamlspecial-case; the same guard exists inscripts/ci_install_tokenspeed.sh).Changes
.github/workflows/release-tokenspeed-docker.yml(new) — thin wrapper over_build-engine-image.yml:engine: tokenspeed, default baselightseekorg/tokenspeed:tml, single-version, PR dry-run,workflow_dispatchoverrides..github/workflows/_build-engine-image.yml— addtokenspeedto the engine validate allowlist + error message + input doc.docker/engine.Dockerfile— addtokenspeedto thecaseallowlist + header doc; droptokenspeed-smg-grpc-{proto,servicer}beforeinstall-smg.sh.scripts/installation/install-tokenspeed.sh(new) — editable install for the optionaltokenspeed_reposource path (parity with the other engines)..github/workflows/nightly-engine-docker.yml— addtokenspeed(base:tml) to the nightly matrix → floatingnightly-tokenspeedtag.Out of scope (follow-up):
smg serve --backend tokenspeedis not yet wired intobindings/python/src/smg/serve.py(BACKEND_CHOICESissglang/vllm/trtllm). The image works in router mode (smg launch→ tokenspeed gRPC workers);SMG_DEFAULT_BACKEND=tokenspeedis set for consistency with the runtime label.Test Plan
Local static validation:
pre-commit run --files <changed files>→ all hooks pass (Rust/Python hooks correctly skipped — no such files changed).install-tokenspeed.shpassesshellcheck+sh -n.Reproducible CI validation (post-merge / on this PR):
docker/engine.Dockerfile,_build-engine-image.yml,release-*-docker.yml, andscripts/installation/**, so thepull_requesttrigger runs the new workflow in dry-run (build-only, no push) — before this PR there was no tokenspeed build; after, the SMG+TokenSpeed image builds againstlightseekorg/tokenspeed:tml.workflow_dispatchon Release SMG+TokenSpeed Docker Image pushesghcr.io/lightseekorg/smg:<smg_ver>-tokenspeed-tml.Checklist
cargo +nightly fmtpasses (N/A — no Rust changed)cargo clippy --all-targets --all-features -- -D warningspasses (N/A — no Rust changed)Summary by CodeRabbit
tokenspeed) as a supported engine option for engine image builds.