-
Notifications
You must be signed in to change notification settings - Fork 693
CI Updates #536
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
CI Updates #536
Conversation
Useful for the asset store releases
…release creation It should work in theory, and correctly release to Docker and pypi via tags.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates CI workflows so that version bumps automatically trigger Docker and PyPI publish workflows, and Docker images are additionally tagged with the major version. Flow diagram for Docker image tagging strategy updateflowchart TD
A[Start publish-docker.yml job] --> B[Docker metadata step]
B --> C[Generate tags from semver]
C --> D[Tag: {{version}}]
C --> E[Tag: {{major}}.{{minor}}]
C --> F[Tag: {{major}}]
C --> G[Tag: branch-based tag]
D --> H[Build and push image]
E --> H
F --> H
G --> H
H --> I[End job]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe changes consolidate Docker and PyPI publishing workflows into reusable composite GitHub Actions, removing standalone publish workflows and integrating publishing operations into the release workflow. The release workflow is renamed from "Bump Version" to "Release" and now orchestrates Docker and PyPI publishing as dependent jobs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
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.
Hey - I've left some high level feedback:
- Instead of shelling out to
gh workflow runwithactions: writepermissions, consider convertingpublish-docker.ymlandpublish-pypi.ymltoworkflow_call-triggered workflows and calling them viauses:so dependencies are explicit and you avoid the extra CLI dependency and token scope. - With the new
type=semver,pattern={{major}}tag added, double-check whether thetype=ref,event=branchtag is still needed; if you no longer want branch-based tags, removing that entry will prevent unexpected extra image tags being published.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of shelling out to `gh workflow run` with `actions: write` permissions, consider converting `publish-docker.yml` and `publish-pypi.yml` to `workflow_call`-triggered workflows and calling them via `uses:` so dependencies are explicit and you avoid the extra CLI dependency and token scope.
- With the new `type=semver,pattern={{major}}` tag added, double-check whether the `type=ref,event=branch` tag is still needed; if you no longer want branch-based tags, removing that entry will prevent unexpected extra image tags being published.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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 @.github/workflows/bump-version.yml:
- Around line 127-137: The current "Trigger publish workflows" step invokes gh
workflow run for publish-docker.yml and publish-pypi.yml (gh workflow run
publish-docker.yml --ref "$TAG" and gh workflow run publish-pypi.yml --ref
"$TAG"), but those workflows are already triggered by on: push: tags: "*" —
remove the explicit dispatch to avoid duplicate runs by deleting the two gh
workflow run lines (or the entire run block if preferred) so the TAG/NEW_VERSION
flow only pushes the tag and relies on the automatic tag-triggered publish
workflows.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/bump-version.yml.github/workflows/publish-docker.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 468
File: Server/DOCKER_OVERVIEW.md:22-22
Timestamp: 2025-12-19T21:09:44.740Z
Learning: In the Unity MCP project, the Docker workflow (.github/workflows/publish-docker.yml) intentionally uses the DOCKER_USERNAME secret instead of hardcoding "msanatan" to allow users who fork the repository to publish to their own Docker Hub repositories, while the documentation (Server/DOCKER_OVERVIEW.md) correctly references the official image location "msanatan/mcp-for-unity-server" for end users.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (2)
.github/workflows/publish-docker.yml (1)
31-31: LGTM! Major version tagging added for user convenience.The addition of
type=semver,pattern={{major}}enables users to pull images using major version tags (e.g.,msanatan/mcp-for-unity-server:1for any 1.x.x version), which is useful for asset store releases as noted in the PR objectives.Note: Major version tags are mutable by design—they'll be updated to point to the latest release within that major version.
.github/workflows/bump-version.yml (1)
22-22: Correct permission for triggering workflows.The
actions: writepermission is necessary for thegh workflow runcommands in the "Trigger publish workflows" step.
…e actions Refactored version bump, Docker publish, and PyPI publish workflows into a unified release pipeline that runs sequentially. Created reusable composite actions for Docker and PyPI publishing to eliminate duplication. Changes: - Rename bump-version.yml to release.yml with three jobs: bump, publish_docker, publish_pypi - Create .github/actions/publish-docker/action.yml with configurable inputs for image, version, branch
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 @.github/actions/publish-pypi/action.yml:
- Around line 6-11: The workflow step installing uv uses version: "latest",
which risks breaking changes; change the Install uv step (uses:
astral-sh/setup-uv@v7) to pin a specific version by replacing version: "latest"
with an explicit pinned version string (e.g., version: "0.9.21") so builds are
reproducible and avoid unexpected uv upgrades.
🧹 Nitpick comments (2)
.github/actions/publish-docker/action.yml (2)
42-96: Consider consolidating metadata steps to reduce duplication.The four conditional metadata steps are functionally correct and mutually exclusive, but result in significant YAML duplication. You could potentially use a single step with dynamically constructed
tagsinput, or use GitHub Actions expressions to conditionally append the branch tag. That said, the current approach is explicit and easy to understand.
65-73: Version parsing assumes no 'v' prefix.The script splits
inputs.versiondirectly on., which works for1.2.3but would break if the caller passesv1.2.3. While the release workflow correctly passes only the numeric version (needs.bump.outputs.new_version), consider adding a strip for robustness:run: | set -euo pipefail + VERSION="${{ inputs.version }}" + VERSION="${VERSION#v}" - IFS='.' read -r MA MI PA <<< "${{ inputs.version }}" + IFS='.' read -r MA MI PA <<< "$VERSION" echo "major=$MA" >> "$GITHUB_OUTPUT" echo "minor=$MI" >> "$GITHUB_OUTPUT"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/actions/publish-docker/action.yml.github/actions/publish-pypi/action.yml.github/workflows/publish-docker.yml.github/workflows/publish-pypi.yml.github/workflows/release.yml
💤 Files with no reviewable changes (2)
- .github/workflows/publish-pypi.yml
- .github/workflows/publish-docker.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 468
File: Server/DOCKER_OVERVIEW.md:22-22
Timestamp: 2025-12-19T21:09:44.740Z
Learning: In the Unity MCP project, the Docker workflow (.github/workflows/publish-docker.yml) intentionally uses the DOCKER_USERNAME secret instead of hardcoding "msanatan" to allow users who fork the repository to publish to their own Docker Hub repositories, while the documentation (Server/DOCKER_OVERVIEW.md) correctly references the official image location "msanatan/mcp-for-unity-server" for end users.
📚 Learning: 2025-12-19T21:09:44.740Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 468
File: Server/DOCKER_OVERVIEW.md:22-22
Timestamp: 2025-12-19T21:09:44.740Z
Learning: In the Unity MCP project, the Docker workflow (.github/workflows/publish-docker.yml) intentionally uses the DOCKER_USERNAME secret instead of hardcoding "msanatan" to allow users who fork the repository to publish to their own Docker Hub repositories, while the documentation (Server/DOCKER_OVERVIEW.md) correctly references the official image location "msanatan/mcp-for-unity-server" for end users.
Applied to files:
.github/actions/publish-docker/action.yml
🔇 Additional comments (12)
.github/actions/publish-docker/action.yml (3)
1-32: Well-structured input definitions with sensible defaults.The inputs are comprehensive and allow for flexibility in different use cases. Using
DOCKER_USERNAMEsecret for the image name (as referenced in the release workflow) maintains fork compatibility. Based on learnings, this aligns with the project's design to allow forked repositories to publish to their own Docker Hub.
36-40: LGTM!Standard Docker Hub login using the official action.
101-111: Build and push configuration looks correct.The fallback chain for
tagsandlabelsworks because exactly one of the four metadata steps will execute based on the mutually exclusive conditions. GHA caching is properly configured for build performance..github/actions/publish-pypi/action.yml (2)
13-16: LGTM!The build step correctly uses the
Server/working directory for the Python package.
18-21: LGTM!Uses PyPI trusted publishing via OIDC, which is the recommended secure approach. The release workflow correctly grants
id-token: writepermission for this to work..github/workflows/release.yml (7)
1-24: Clean workflow structure with proper job outputs.The workflow is well-organized with clear job dependencies. The outputs
new_versionandtagallow downstream jobs to reference the version correctly.
31-69: LGTM!The version computation logic is correct, handles all three bump types properly, and uses strict bash error handling.
71-106: LGTM!File updates are handled correctly. The sed patterns appropriately update version references, and the commit logic skips if no changes are detected.
107-121: Idempotent tag handling is good, but consider the existing tag scenario.The check for existing tags prevents failures on re-runs. However, if a tag already exists pointing to a different commit, the publish jobs will checkout and publish from that older commit. If this is intentional idempotent behavior, consider adding a comment. If not, you may want to fail explicitly when the tag exists but points to a different SHA.
123-128: LGTM!The
softprops/action-gh-releaseaction is a solid choice with automatic release notes generation.
130-154: Docker publish job is correctly configured.Good use of the tag-based checkout to ensure the Docker image is built from the released code. The use of
${{ secrets.DOCKER_USERNAME }}for the image namespace maintains fork compatibility as per project conventions.Minor note:
fetch-depth: 0isn't strictly necessary for building a Docker image since you're already checking out a specific tag. You could usefetch-depth: 1for faster checkout, but this is a minor optimization.
156-174: PyPI publish job is well-configured with trusted publishing.The
id-token: writepermission enables OIDC-based trusted publishing to PyPI, eliminating the need for stored API tokens. Theenvironment: pypiprovides additional protection through environment approval rules if configured.
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v7 | ||
| with: | ||
| version: "latest" | ||
| enable-cache: true | ||
| cache-dependency-glob: "Server/uv.lock" |
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.
🧩 Analysis chain
🌐 Web query:
astral-sh setup-uv latest stable version best practices
💡 Result:
Short answer
- Action major: use astral-sh/setup-uv@v7 (current stable major). [1][2]
- uv binary: pin the uv release you need (e.g., version: "0.9.21") rather than relying on latest. [2][3]
Best-practice checklist (concise)
- Pin the action major and pin uv:
- uses: astral-sh/setup-uv@v7
- with: version: "0.9.21" (or the exact uv release you require). [1][2][3]
- Run actions/checkout before setup-uv when you need repo files (pyproject.toml / uv.toml). [1]
- Use working-directory if your project lives in a subfolder so version files are discovered there. [1]
- Enable caching and include resolution strategy in the cache key (or use the action’s caching guidance) to avoid cache conflicts. Add cache-suffix/resolution-strategy to cache keys. [1]
- Set python-version or run uv python install explicitly (don’t assume actions/setup-python unless you prefer it for speed). [1]
- Use the action outputs (uv-version, uv-path) when subsequent steps need the installed version. [1]
- (Optional) Verify checksum input for supply-chain safety and set github-token for rate limits when needed. [1]
- Persist or copy uv into Docker images by pinning the distroless image tag (e.g., ghcr.io/astral-sh/uv:0.9.21) for reproducible builds. [3]
Minimal example
- uses: actions/checkout@v5
- name: Install uv
uses: astral-sh/setup-uv@v7
with:
version: "0.9.21"
python-version: "3.12"
enable-cache: true
Sources
[1] astral-sh/setup-uv (GitHub README).
[2] Using uv in GitHub Actions — docs.astral.sh/uv/guides/integration/github.
[3] Using uv in Docker — docs.astral.sh/uv/guides/integration/docker.
🌐 Web query:
uv Python package manager latest stable release version 2025
💡 Result:
The latest stable UV release in 2025 is version 0.9.21 (released Dec 30, 2025). [1][2]
Sources:
[1] PyPI — uv release history (uv‑0.9.21 uploaded Dec 30, 2025).
[2] GitHub — astral‑sh/uv releases (0.9.20 notes; repo releases list).
🌐 Web query:
astral-sh setup-uv action breaking changes version pinning recommendations
💡 Result:
Short summary of recent breaking changes and pinning recommendations for astral-sh/setup-uv
Breaking changes to be aware of
- setup-uv v7: switched runtime from Node 20 → Node 24 and removed the deprecated server-url input (breaking for out-of-date self-hosted runners or workflows relying on server-url). [1]
- uv 0.9.x (upstream) made Python 3.14 the default stable Python and noted potential breaking behavior if you do not pin Python versions. (This is an upstream uv release note you should consider when installing uv in CI.) [2]
Recommended version-pinning and workflow practices
- Pin the action major tag (and optionally patch) — e.g. uses: astral-sh/setup-uv@v7 — to avoid unexpected breaking changes from new major releases. [1]
- Pin the uv binary version explicitly in the action inputs: with: version: "0.9.18" (or whatever exact uv release you need). This prevents automatic upgrades when the action installs the “latest”. [3][1]
- Pin the Python version you want uv to use:
- Either set with: python-version: "3.13" (or desired) in setup-uv, or
- Use actions/setup-python before setup-uv and set python-version-file / python-version explicitly. This avoids surprising default upgrades (e.g., to Python 3.14). [1][2]
- Use a version-file (uv.toml or pyproject.toml) or the action input version-file to centralize the pinned uv version; set resolution-strategy if using ranges. [1]
- If you rely on caching or self-hosted runners, ensure runners/tools meet v7’s Node 24 requirement; update self-hosted runners if necessary. [1][3]
- For safety in CI: use the action output uv-version to assert the expected installed version in later steps, and enable checksum/manifest options if you download from custom sources. [1][3]
Useful references
- Using uv with GitHub Actions / docs (examples, version-file, python-version): astral docs. [1]
- setup-uv Marketplace / inputs & behavior. [3]
- setup-uv releases (v7 breaking notes). [3]
- uv releases (upstream breaking change: Python 3.14 default). [2]
If you want, I can draft the minimal workflow snippet showing recommended pinning (action + uv version + python-version + assertion).
Pin uv to a specific version (e.g., 0.9.21) instead of "latest" for reproducibility.
Using version: "latest" can lead to unexpected build failures when uv releases introduce breaking changes. Recent versions have introduced breaking behavior (e.g., uv 0.9.x changed the default Python version to 3.14). Pin to an exact version like version: "0.9.21" to ensure consistent and predictable builds, aligning with your stability requirements.
🤖 Prompt for AI Agents
In @.github/actions/publish-pypi/action.yml around lines 6 - 11, The workflow
step installing uv uses version: "latest", which risks breaking changes; change
the Install uv step (uses: astral-sh/setup-uv@v7) to pin a specific version by
replacing version: "latest" with an explicit pinned version string (e.g.,
version: "0.9.21") so builds are reproducible and avoid unexpected uv upgrades.
|
@dsarno future updates should be automatic in future. Just flag me if it fails and I'm not online |
|
Nice!! |
Summary by Sourcery
Automate post-version-bump publication workflows and adjust Docker image tagging for major-version tags.
Build:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.