feat(system): support custom brew taps#10375
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesHomebrew Taps Configuration and Installation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR adds third-party Homebrew tap support to mise's system package manager: a declarative
Confidence Score: 4/5Safe to merge after addressing the core-formula install ordering fix; tapped-formula path is well-contained and does not regress existing homebrew/core behaviour. The install and upgrade methods perform an early brew-binary check before installing core formulae. When a user mixes core and tapped packages and the brew binary is absent, the check fails and core formulae — which never need brew — are silently skipped. The check inside install_via_brew / upgrade_via_brew already covers this, so the early guards are redundant and can be dropped without any other changes. src/system/packages/brew/mod.rs — the install and upgrade method ordering Important Files Changed
Reviews (5): Last reviewed commit: "feat(system): support custom brew taps" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/system-packages/brew.md (1)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the opening statement conditional.
Line 3 says Homebrew is not required, but tapped flows later in this page require a real
brew. Tighten this sentence to explicitly scope the “no Homebrew required” claim tohomebrew/coreformulae.🤖 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 `@docs/system-packages/brew.md` at line 3, The opening claim "Homebrew formulae — **without requiring Homebrew to be installed**." is too broad; limit the scope to only homebrew/core formulae. Edit that header/text to read something like "Homebrew formulae from homebrew/core — without requiring Homebrew to be installed." and add a short qualifier sentence noting that tapped formulae and flows that reference third‑party taps still require a real brew installation (so readers know why later "tapped flows" need brew). Ensure you update the exact string "Homebrew formulae — **without requiring Homebrew to be installed**." and any nearby sentences referencing "tapped flows" to reflect this narrower scope.
🧹 Nitpick comments (1)
src/system/mod.rs (1)
237-250: ⚡ Quick winUnify tap-name parsing in one shared helper.
brew_tap_nameis implemented here and again insrc/system/packages/brew/mod.rs. If they drift, config-timetap_urlattachment and runtime tapped/core routing will disagree. Please centralize this parser and reuse it in both places.🤖 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 `@src/system/mod.rs` around lines 237 - 250, The brew_tap_name parsing logic is duplicated (one implementation in brew_tap_name and another in the Brew package module); extract a single shared helper (e.g., pub(crate) fn parse_brew_tap_name or make brew_tap_name pub(crate)) and replace the duplicate implementation in the packages/brew module to call that centralized function, adjusting visibility and imports as needed so both callers use the same logic and behavior (retain the exact current semantics: split on '/', require exactly three parts, treat "homebrew/core" as None, and return the tap via rsplit_once).
🤖 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 `@src/system/packages/brew/mod.rs`:
- Around line 391-396: The subprocess awaits in run_brew and brew_list_version
can hang indefinitely; wrap the .status().await calls in tokio::time::timeout
with a sensible Duration (e.g. from a constant or config) and map timeout errors
to a clear error message; update the code around the calls in run_brew and
brew_list_version (identify the Command::new(brew) ... .status().await
sequences) to use timeout(...).await, convert Err(elapsed) into a descriptive
wrap_err_with like "brew timed out after <duration> running <display_cmd(...)>"
and preserve existing error wrapping for non-timeout failures.
---
Outside diff comments:
In `@docs/system-packages/brew.md`:
- Line 3: The opening claim "Homebrew formulae — **without requiring Homebrew to
be installed**." is too broad; limit the scope to only homebrew/core formulae.
Edit that header/text to read something like "Homebrew formulae from
homebrew/core — without requiring Homebrew to be installed." and add a short
qualifier sentence noting that tapped formulae and flows that reference
third‑party taps still require a real brew installation (so readers know why
later "tapped flows" need brew). Ensure you update the exact string "Homebrew
formulae — **without requiring Homebrew to be installed**." and any nearby
sentences referencing "tapped flows" to reflect this narrower scope.
---
Nitpick comments:
In `@src/system/mod.rs`:
- Around line 237-250: The brew_tap_name parsing logic is duplicated (one
implementation in brew_tap_name and another in the Brew package module); extract
a single shared helper (e.g., pub(crate) fn parse_brew_tap_name or make
brew_tap_name pub(crate)) and replace the duplicate implementation in the
packages/brew module to call that centralized function, adjusting visibility and
imports as needed so both callers use the same logic and behavior (retain the
exact current semantics: split on '/', require exactly three parts, treat
"homebrew/core" as None, and return the tap via rsplit_once).
🪄 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: Pro Plus
Run ID: 29e8ba4d-c2f1-4a3f-b027-e90dd1b2d075
📒 Files selected for processing (8)
docs/system-packages/brew.mdsrc/config/config_file/mise_toml.rssrc/system/mod.rssrc/system/packages/apt.rssrc/system/packages/brew/mod.rssrc/system/packages/dnf.rssrc/system/packages/mod.rssrc/system/packages/pacman.rs
jdx
force-pushed
the
codex/custom-brew-taps
branch
from
4d24fde to
d45c692
Compare
jdx
force-pushed
the
codex/custom-brew-taps
branch
from
d45c692 to
010d2aa
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/cli/system/use.rs (1)
56-56: ⚡ Quick winImport
Configfor consistency.
install.rsandupgrade.rsboth importConfigfromcrate::config, but this file uses the fully qualified path. For consistency and brevity, consider addingConfigto the existing import at line 9.♻️ Proposed fix
-use crate::config::{ConfigPathOptions, Settings, resolve_target_config_path}; +use crate::config::{Config, ConfigPathOptions, Settings, resolve_target_config_path};Then update line 56:
- let config = crate::config::Config::get().await?; + let config = Config::get().await?;🤖 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 `@src/cli/system/use.rs` at line 56, The code uses the fully-qualified call crate::config::Config::get() in use.rs; add Config to the existing imports (so Config is brought into scope) and then replace the fully-qualified call with Config::get(). In other words, update the imports to include the Config type (matching install.rs/upgrade.rs) and change the call site (the expression using crate::config::Config::get()) to Config::get() to keep style consistent and concise.src/system/mod.rs (2)
137-145: ⚡ Quick winMake
attach_brew_tap_urlsadditive.This helper currently overwrites any pre-set
request.tap_urlwith the config lookup result. If a caller ever passes already-resolved brew requests through it, an unmatched config entry will clear that metadata back toNone. Only fill missing values here so the helper behaves like “attach”, not “replace”.♻️ Suggested change
pub fn attach_brew_tap_urls(config: &Config, by_mgr: &mut IndexMap<String, Vec<PackageRequest>>) { let brew_taps = brew_taps_from_config(config); let Some(requests) = by_mgr.get_mut("brew") else { return; }; for request in requests { - request.tap_url = brew_tap_name(&request.name).and_then(|tap| brew_taps.get(tap).cloned()); + if request.tap_url.is_none() { + request.tap_url = + brew_tap_name(&request.name).and_then(|tap| brew_taps.get(tap).cloned()); + } } }🤖 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 `@src/system/mod.rs` around lines 137 - 145, The function attach_brew_tap_urls currently unconditionally sets request.tap_url from the config which can overwrite an already-resolved tap_url; change the logic so it only fills missing values: inside attach_brew_tap_urls (operating on requests from by_mgr.get_mut("brew")), check request.tap_url.is_none() before calling brew_tap_name(&request.name) and assigning from brew_taps.get(tap).cloned(), leaving existing tap_url values intact; keep using brew_taps_from_config and brew_tap_name as the lookup helpers.
359-368: ⚡ Quick winAdd one regression test for tap-url attachment, not just tap parsing.
The new behavior here is the URL attachment path used by
mise system useand explicitmise system install, but the added test only exercisesbrew_tap_name()in isolation. A focused test that provesowner/tap/formulapicks up the configured URL whilehomebrew/core/*stays unset would lock down the cross-file contract more effectively. Based on the install/use call paths in the provided snippets and the PR objective for config-driven tap resolution.🤖 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 `@src/system/mod.rs` around lines 359 - 368, Add a regression test that not only asserts brew_tap_name() parsing but also exercises the higher-level tap URL attachment path used by the CLI commands ("mise system use" / "mise system install") to ensure owner/tap/formula picks up a configured tap URL while homebrew/core/* remains unset; in practice add a test which calls the same install/use code path (the function invoked by those commands) with a configured tap URL for "railwaycat/emacsmacport/emacs-mac" and assert the resolved attachment URL is present, then call the same path for "homebrew/core/jq" and "jq" and assert their attachment URL is None, keeping brew_tap_name() assertions as part of the test to lock the cross-file contract.
🤖 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.
Nitpick comments:
In `@src/cli/system/use.rs`:
- Line 56: The code uses the fully-qualified call crate::config::Config::get()
in use.rs; add Config to the existing imports (so Config is brought into scope)
and then replace the fully-qualified call with Config::get(). In other words,
update the imports to include the Config type (matching install.rs/upgrade.rs)
and change the call site (the expression using crate::config::Config::get()) to
Config::get() to keep style consistent and concise.
In `@src/system/mod.rs`:
- Around line 137-145: The function attach_brew_tap_urls currently
unconditionally sets request.tap_url from the config which can overwrite an
already-resolved tap_url; change the logic so it only fills missing values:
inside attach_brew_tap_urls (operating on requests from by_mgr.get_mut("brew")),
check request.tap_url.is_none() before calling brew_tap_name(&request.name) and
assigning from brew_taps.get(tap).cloned(), leaving existing tap_url values
intact; keep using brew_taps_from_config and brew_tap_name as the lookup
helpers.
- Around line 359-368: Add a regression test that not only asserts
brew_tap_name() parsing but also exercises the higher-level tap URL attachment
path used by the CLI commands ("mise system use" / "mise system install") to
ensure owner/tap/formula picks up a configured tap URL while homebrew/core/*
remains unset; in practice add a test which calls the same install/use code path
(the function invoked by those commands) with a configured tap URL for
"railwaycat/emacsmacport/emacs-mac" and assert the resolved attachment URL is
present, then call the same path for "homebrew/core/jq" and "jq" and assert
their attachment URL is None, keeping brew_tap_name() assertions as part of the
test to lock the cross-file contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f899d94-4703-43af-a97a-3872747944b7
📒 Files selected for processing (21)
docs/cli/index.mddocs/cli/system.mddocs/cli/system/brew.mddocs/cli/system/brew/tap.mddocs/cli/system/brew/untap.mddocs/system-packages/brew.mdmise.usage.kdlsrc/cli/system/brew/mod.rssrc/cli/system/brew/tap.rssrc/cli/system/brew/untap.rssrc/cli/system/install.rssrc/cli/system/mod.rssrc/cli/system/upgrade.rssrc/cli/system/use.rssrc/config/config_file/mise_toml.rssrc/system/mod.rssrc/system/packages/apt.rssrc/system/packages/brew/mod.rssrc/system/packages/dnf.rssrc/system/packages/mod.rssrc/system/packages/pacman.rs
✅ Files skipped from review due to trivial changes (5)
- src/system/packages/dnf.rs
- docs/cli/system/brew/tap.md
- docs/cli/system.md
- docs/cli/index.md
- docs/cli/system/brew/untap.md
🚧 Files skipped from review as they are similar to previous changes (11)
- src/system/packages/pacman.rs
- src/system/packages/apt.rs
- src/system/packages/mod.rs
- docs/cli/system/brew.md
- docs/system-packages/brew.md
- src/config/config_file/mise_toml.rs
- src/cli/system/brew/tap.rs
- src/cli/system/brew/untap.rs
- src/cli/system/brew/mod.rs
- mise.usage.kdl
- src/system/packages/brew/mod.rs
jdx
force-pushed
the
codex/custom-brew-taps
branch
from
010d2aa to
aa5e948
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit aa5e948. Configure here.
jdx
force-pushed
the
codex/custom-brew-taps
branch
from
aa5e948 to
1c0d937
Compare
jdx
force-pushed
the
codex/custom-brew-taps
branch
from
1c0d937 to
5eb0471
Compare
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.5 x -- echo |
17.9 ± 0.8 | 16.4 | 21.1 | 1.00 |
mise x -- echo |
18.7 ± 1.6 | 17.2 | 44.0 | 1.05 ± 0.10 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.5 env |
17.8 ± 0.8 | 16.2 | 22.9 | 1.00 |
mise env |
18.5 ± 0.9 | 16.9 | 22.6 | 1.04 ± 0.07 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.5 hook-env |
18.3 ± 0.8 | 16.9 | 24.4 | 1.00 |
mise hook-env |
19.1 ± 0.8 | 17.5 | 23.1 | 1.04 ± 0.06 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.5 ls |
14.9 ± 0.7 | 13.6 | 18.6 | 1.00 |
mise ls |
15.6 ± 0.7 | 14.1 | 21.2 | 1.04 ± 0.07 |
xtasks/test/perf
| Command | mise-2026.6.5 | mise | Variance |
|---|---|---|---|
| install (cached) | 132ms | 132ms | +0% |
| ls (cached) | 57ms | 59ms | -3% |
| bin-paths (cached) | 62ms | 65ms | -4% |
| task-ls (cached) | 123ms | 124ms | +0% |
1 participant
Summary
[system.brew.taps]for declaring Homebrew tap remotes, similar to[plugins]mise system brew tap/mise system brew untapcommands that shell out to Homebrew without modifyingmise.tomlbrewcommandbrew tapandbrew update-if-neededbefore installing or upgrading tapped formulaehomebrew/coreformulae on mise’s direct bottle/source installer pathbrew list --versionssafelyValidation
cargo fmt --allmise run lintcargo test test_brew_tap_namecargo test test_tapped_formula_detectioncargo test test_system_packagescargo check --all-featurescargo buildmise run render:usageMISE_EXPERIMENTAL=1 target/debug/mise system install brew:acme/tools/widget --dry-run --yesMISE_EXPERIMENTAL=1 target/debug/mise system brew tap acme/tools https://git.example.com/acme/homebrew-tools.git --dry-runMISE_EXPERIMENTAL=1 target/debug/mise system brew untap acme/tools --dry-rungit diff --checkNote
Medium Risk
Changes brew system-package install/upgrade paths and runs timed
brewsubprocesses; scope is experimental system packages, not auth, but mis-tapped installs could affect the host Homebrew environment.Overview
Adds third-party Homebrew tap support for experimental
[system.packages]brew entries while keeping homebrew/core on mise’s direct bottle/source installer.Declarative config gains
[system.brew.taps](owner/tap→ git URL, like[plugins]). Tapped formulae use names likebrew:owner/tap/formula; install/upgrade/status attachtap_urlfrom config, runbrew tap(with URL when needed) andbrew update-if-needed, then delegatebrew install/brew upgradefor tapped packages only. Core formulae still use the existing pour path and do not shell out tobrew.New Unix-only commands
mise system brew tapanduntap(with--dry-run) call Homebrew directly and do not editmise.toml.mise system install/upgrade/useload config when resolving explicit package specs so tap URLs apply there too.Docs, usage spec, man page, and Fig metadata are updated; limitations now note tapped formulae require a real Homebrew install.
Reviewed by Cursor Bugbot for commit 5eb0471. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes