Skip to content

fix(system): remove brew cli proxying#10381

Closed
jdx wants to merge 1 commit into
mainfrom
codex/brew-cask-system-packages
Closed

fix(system): remove brew cli proxying#10381
jdx wants to merge 1 commit into
mainfrom
codex/brew-cask-system-packages

Conversation

@jdx

@jdx jdx commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • remove mise system brew tap/untap, which proxied to the Homebrew CLI
  • remove tapped-formula install/upgrade/status shell-outs to brew
  • make third-party tapped formula installs fail explicitly instead of delegating
  • update docs and generated usage to state that unsupported Homebrew features do not proxy to brew

Validation

  • cargo fmt --all -- --check
  • cargo check --all-features
  • mise run render:usage
  • mise run test:e2e cli/test_system_status
  • mise run test:unit test_parse_use_spec
  • mise run test:unit test_brew_tap_name
  • mise run test:unit test_system_packages
  • git diff --check

This PR was generated by an AI coding assistant.

Summary by CodeRabbit

  • Breaking Changes

    • Removed mise system brew tap and mise system brew untap commands.
    • Homebrew support now limited to core formulae only; third-party taps will fail with an explicit error.
  • New Features

    • Added brew-cask as a package manager option for mise system install and mise system upgrade.
  • Documentation

    • Updated Homebrew system-packages guide to clarify supported features and limitations.

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR removes custom Homebrew tap support and restricts BrewManager to only homebrew/core formulae. It deletes the tap_url field from system package requests, removes mise system brew tap/untap CLI commands, eliminates all Homebrew subprocess proxy paths, and updates documentation to clarify the scope. The CLI usage specs are updated to include brew-cask manager support.

Changes

Restrict Brew to Core and Remove Custom Taps

Layer / File(s) Summary
Remove tap_url field from PackageRequest
src/system/packages/mod.rs
PackageRequest struct deletes the tap_url: Option<String> field, narrowing the request payload to name and version only.
Remove tap URL derivation and wiring from system module
src/system/mod.rs
Deletes attach_brew_tap_urls and brew_taps_from_config functions and removes all tap URL derivation logic from parse_use_spec, packages_from_config_files, and packages_from_specs_with_config so system package requests no longer read or compute tap URLs.
Restrict BrewManager to homebrew/core formulae
src/system/packages/brew/mod.rs
Updates BrewManager to support only homebrew/core by removing all Homebrew subprocess paths (install_via_brew, upgrade_via_brew, brew_list_version), changing installed() to use only pour::linked_version, and adding bail_unsupported_taps() to reject tapped requests. Module docs clarify casks and third-party taps are not implemented.
Remove mise system brew command from CLI surface
src/cli/system/mod.rs
Deletes the Unix-only brew module declaration, removes the Commands::Brew(...) enum variant, and removes the corresponding dispatch branch so mise system no longer exposes tap and untap subcommands.
Simplify SystemUse flow by removing tap URL loading
src/cli/system/use.rs
SystemUse::run no longer calls Config::get() or system::attach_brew_tap_urls() before manager resolution.
Update test fixtures to match new PackageRequest shape
src/system/packages/apt.rs, src/system/packages/dnf.rs, src/system/packages/pacman.rs
Removes tap_url: None field from PackageRequest test helper functions.
Update user documentation to reflect brew-only-core scope
docs/system-packages/brew.md
Clarifies only homebrew/core formulae are supported, third-party taps are not installed, [system.brew.taps] is reserved for future direct support without proxying to brew, and adds "No brew executable proxying" limitation.
Remove tap/untap command references from CLI documentation index
docs/cli/index.md, docs/cli/system.md
Removes references to mise system brew and its subcommands from the CLI documentation index pages.
Add brew-cask to CLI usage specs and add unsupported-tap e2e test
mise.usage.kdl, e2e/cli/test_system_status
Extends mise system install and mise system upgrade CLI specs to include brew-cask in manager choices and examples; adds e2e test asserting that tapped formula installs fail with refusal-to-proxy error.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • jdx/mise#10375: Adds custom-tap support with tap_url field and mise system brew tap/untap commands; this PR reverses those changes.
  • jdx/mise#10346: Both PRs modify parse_use_spec and PackageRequest handling in src/system/mod.rs.
  • jdx/mise#10364: Both PRs modify the BrewManager install/upgrade flow in src/system/packages/brew/mod.rs.

🐰 No more taps and proxy calls,
Core formulae only now—simplicity's call,
Casks get their own lane, bright and clear,
Less complexity, more cheer!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'fix(system): remove brew cli proxying' accurately describes the main change: removing Homebrew CLI proxying for unsupported brew features (taps, casks, services), which is the central theme across multiple file changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread src/system/packages/brew/mod.rs Outdated
@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes the previous brew tap-proxy behaviour — tapped formulae (owner/tap/formula) and the mise system brew tap/untap CLI commands are deleted in favour of a hard error that explicitly refuses to shell out to brew. tap_url is removed from PackageRequest, the attach_brew_tap_urls helper is gone, and SystemBrewTomlConfig is kept (for forward-compatible TOML parsing) but annotated #[allow(dead_code)].

  • Tapped formulae now bail! with an "unsupported / refusing to proxy" message instead of delegating to a real brew install.
  • PackageRequest is simplified by dropping the tap_url field; all four managers (apt, brew, dnf, pacman) and test helpers are updated accordingly.
  • mise system brew tap / untap subcommands and their docs are removed; the e2e test and docs are updated to match the new no-proxy policy.

Confidence Score: 4/5

Safe to merge for users with homebrew/core-only configs; users who have tapped formulae alongside core formulae in the same [system.packages] block will find that core installs are blocked too.

In both install and upgrade, bail_unsupported_taps fires before the core-formula path runs, so a config mixing brew:jq with brew:owner/tap/pkg fails to install jq at all.

src/system/packages/brew/mod.rs — the install/upgrade ordering means tapped formulae block core formulae from being processed.

Important Files Changed

Filename Overview
src/system/packages/brew/mod.rs Removes brew-tap proxy and brew-list-version; adds bail_unsupported_taps, but bails before core formulae are installed when tapped formulae are present — breaking mixed configs
src/system/mod.rs Removes tap_url, brew_taps_from_config, attach_brew_tap_urls, and associated tap-URL plumbing; packages_from_specs_with_config retains an unused _config parameter
src/system/packages/mod.rs Removes tap_url field from PackageRequest — clean struct simplification, all call sites updated
src/cli/system/use.rs Removes the now-unused Config load and attach_brew_tap_urls call; logic is clean
src/cli/system/mod.rs Removes the mise system brew subcommand tree (tap/untap) from the CLI router
e2e/cli/test_system_status Adds e2e assertion that tapped formulae now fail with the "refusing to proxy" message
docs/system-packages/brew.md Documentation updated to reflect the no-proxy policy for tapped formulae and to remove tap management commands

Fix All in Claude Code

Reviews (2): Last reviewed commit: "fix(system): remove brew cli proxying" | Re-trigger Greptile

Comment thread src/system/packages/brew/mod.rs Outdated
@github-actions

github-actions Bot commented Jun 13, 2026

Copy link
Copy Markdown

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.5 x -- echo 18.0 ± 0.7 16.6 21.1 1.00
mise x -- echo 18.7 ± 1.4 17.1 42.1 1.04 ± 0.09

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.5 env 18.2 ± 0.9 16.3 22.9 1.00
mise env 18.5 ± 0.8 16.8 22.8 1.02 ± 0.06

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.5 hook-env 18.6 ± 0.7 16.9 22.5 1.00
mise hook-env 19.2 ± 0.7 17.7 23.0 1.03 ± 0.06

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.5 ls 15.0 ± 0.6 13.6 17.6 1.00
mise ls 15.7 ± 0.7 14.2 18.9 1.04 ± 0.06

xtasks/test/perf

Command mise-2026.6.5 mise Variance
install (cached) 132ms 132ms +0%
ls (cached) 59ms 59ms +0%
bin-paths (cached) 62ms 63ms -1%
task-ls (cached) 125ms 127ms -1%

@jdx jdx force-pushed the codex/brew-cask-system-packages branch from 3bd88a5 to b7415a3 Compare June 13, 2026 06:36
@jdx jdx changed the title feat(system): add brew cask packages fix(system): remove brew cli proxying Jun 13, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b7415a3. Configure here.

self.install_via_pour(&core, opts).await?;
}
self.upgrade_via_brew(&tapped, opts).await
Ok(())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tapped brew blocks core installs

Medium Severity

When one brew install or upgrade batch mixes third-party tap formulae with homebrew/core packages, bail_unsupported_taps fails the entire brew manager call before any core pour runs, so valid core packages in the same config or CLI invocation never install or upgrade.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b7415a3. Configure here.

jdx commented Jun 13, 2026

Copy link
Copy Markdown
Owner Author

Closing this PR because its scope drifted in the wrong direction. I will replace it with a single PR that implements direct tap and cask support without proxying to the brew CLI.\n\nThis comment was generated by an AI coding assistant.

@jdx jdx closed this Jun 13, 2026
Comment on lines 250 to 263
@@ -299,13 +259,13 @@ impl SystemPackageManager for BrewManager {
.collect::<Vec<_>>();
self.install_via_pour(&core, opts).await?;
}
self.install_via_brew(&tapped, opts).await
Ok(())
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Tapped-formula error blocks core-formula install in mixed configs

bail_unsupported_taps is called before core formulae are processed. A config containing both "brew:jq" = "latest" and "brew:owner/tap/formula" = "latest" will fail entirely — jq never gets installed — because the bail for the tapped formula short-circuits the core path. The same ordering issue exists in upgrade. Processing core formulae first (or skipping/warning on unsupported tapped ones) would avoid leaving a machine in a half-installed state.

Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant