fix(installer,skills): make uv a real requirement and stop assuming a system Python - #2704
Merged
Conversation
… system Python The installer told users uv was optional while bmad-build had already made it mandatory. uv-check.js called it "becoming the de facto standard", install-messages.yaml led with HEADS UP, and installer.js printed a Tip inside a box titled "BMAD is ready to use!" — while bmad-build and bmad-build-auto HALT on activation without `uv run`. The probe's result was discarded (`await checkUvEnvironment();`), so nothing branched on it. Messaging now names the consequence, and the post-install summary repeats the warning when it applies — the pre-install probe fires before every prompt, so by then it is far up the scrollback. Still warn-don't-block: core-only, docs-only, and CI installs never render a skill, so a missing uv must not fail the run. Adds a python3 probe used only when uv is absent, since that is the only case where the interpreter on PATH matters. It reports whether the direct-interpreter skills still work (3.11+) or nothing Python-backed will (below 3.11, or no python3 at all). Separately, 25 call sites still ran resolve_customization.py under a bare `python3`. That script requires 3.11+ for tomllib, so on macOS without Homebrew or Ubuntu 22.04 they fell through to their "if the script fails" path and hand-merged the TOML in-context — no error surfaced. All 25 now use `uv run`, which provisions a matching interpreter from the script's own requires-python. Four more spawned Python purely to open an HTML file: python3 -c "import webbrowser, pathlib; webbrowser.open(...)" Replaced with the platform opener bmad-brainstorming already uses — open / xdg-open / start. src/ now contains no bare Python invocation at all, so "Python 3.11+" leaves the user contract: uv provisions its own. docs/how-to/customize-bmad.md described a transition that this ends. Test suite 46 grows from 12 to 29 assertions: Python parsing, the 3.11 boundary in both directions, that uv-present skips the python3 probe, and all three missing-uv sub-branches.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (31)
📝 WalkthroughWalkthroughThe installer now detects uv and Python versions, reports fallback conditions, and updates setup messages. Agent and workflow instructions use Changesuv Execution and Installer Support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Installer
participant uvCheckEnvironment
participant uv
participant Python3
Installer->>uvCheckEnvironment: check runtime availability
uvCheckEnvironment->>uv: probe uv version
uv-->>uvCheckEnvironment: uv result
alt uv is unavailable
uvCheckEnvironment->>Python3: probe Python version
Python3-->>uvCheckEnvironment: Python result
end
uvCheckEnvironment-->>Installer: status and fallback details
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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.
The installer told users
uvwas optional whilebmad-buildhad already made it mandatory.The contradiction
bmad-build/SKILL.md:13— "On failure (includinguvbeing unavailable), report the command output and HALT."Meanwhile:
tools/installer/core/uv-check.js:84tools/installer/install-messages.yaml:15tools/installer/core/installer.js:1249And the probe's result was discarded —
ui.js:216wasawait checkUvEnvironment();with no branch on{status: 'missing'}. A user could install, see green, run the headline skill of the release, and hit a halt.What changed
Messaging names the consequence. The missing-uv warning says
bmad-buildandbmad-build-autoHALT, and that there is no interpreter fallback.installer.jsrepeats it in the post-install summary when it applies — the pre-install probe fires before every prompt, so by then it is far up the scrollback.Still warn-don't-block. Core-only, docs-only, and CI installs never render a skill, so a missing
uvmust not fail the run. No new prompt, no non-zero exit.A python3 probe, used only when uv is absent — the only case where the interpreter on PATH matters, since uv otherwise provisions its own. It distinguishes three states: 3.11+ present (direct-interpreter skills still work), below 3.11, and no python3 at all.
The other half: 25 call sites
resolve_customization.pywas still invoked as barepython3in the five agents and six skills. That script requires 3.11+ fortomllib, so on macOS without Homebrew or Ubuntu 22.04 they fell through to their "if the script fails" path and hand-merged the TOML in-context — quietly, with no signal. All 25 now useuv run.Four more spawned Python purely to open an HTML file:
Replaced with the platform opener
bmad-brainstormingalready uses —open/xdg-open/start.src/now contains no bare Python invocation at all. That is the point: "Python 3.11+" can leave the user contract entirely, because uv provisions its own from each script'srequires-python. Verified:docs/how-to/customize-bmad.mddescribed a transition that this ends; its three passages are updated.Tests
Suite 46 grows from 12 to 29 assertions — Python version parsing, the 3.11 boundary in both directions, a future 4.x, that uv-present skips the python3 probe, and all three missing-uv sub-branches. Full suite passes: 473 installer component assertions, lint, markdownlint, prettier.
Follow-ups not in this PR
README.md:13still says Python 3.10+. After this it should say Node + uv, with no Python entry.detectPython3is now unreachable in a fresh install. It is a transitional net for installs upgrading from 6.10 that still have the old skill files on disk; delete it once those are gone.Part of an ecosystem-wide pass — the same conversion is going into bmad-builder, creative-intelligence-suite, game-dev-studio, and test-architecture-enterprise.