Skip to content

fix(brew-cask): drop stale brew advice from the cask metadata error - #12800

Open
Marukome0743 wants to merge 1 commit into
jdx:mainfrom
Marukome0743:brew-cask-stale-tap-advice
Open

fix(brew-cask): drop stale brew advice from the cask metadata error#12800
Marukome0743 wants to merge 1 commit into
jdx:mainfrom
Marukome0743:brew-cask-stale-tap-advice

Conversation

@Marukome0743

@Marukome0743 Marukome0743 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #12645, which I got merged and which has been overtaken by the tap Ruby fallback. Not in a release yet — #12645 merged 2026-09-04, the newest release is v2026.9.1 — so the broken state can be fixed before it ships.

What went stale

#12645 rewrote two brew error messages to say that taps mostly do not publish API metadata and that the user should install with brew. Direct tap support then landed, and docs/bootstrap/packages/brew.md now documents the opposite:

mise first looks for published Homebrew API metadata … When a tap does not publish it, mise fetches the Ruby definition at a pinned tap commit and evaluates its metadata with mise's own Formula or Cask DSL shim.

The formula side was reorganized at the same time and my message there was removed. api.rs now reads:

// inner: short context only
.wrap_err_with(|| format!("failed to fetch Homebrew formula '{name}'"))

// outer: what to do when both halves failed
"failed to resolve Homebrew tap formula '{name}'; published API metadata was unavailable \
 ({api_err}) and mise could not evaluate Formula/{formula_name}.rb"

The cask side was not. fetch_cask_url still carries the long message, and fetch_cask wraps it with the equivalent Ruby-fallback explanation — so the inner text tells the user mise will not proxy to brew from inside the very attempt that goes on to evaluate Casks/<token>.rb.

The broken link

The message ends with https://mise.jdx.dev/bootstrap/packages/brew.html#third-party-taps. That anchor does not exist. brew.md on main has: Casks, Supported platforms, The prefix, Coexistence with a real Homebrew, Importing and pruning, How pouring works, Source formulae, Upgrades, Limitations.

@coderabbitai flagged this on #12645 and I was wrong to push back on it. My argument was that #12644 would add the section and would merge first. #12644 is being closed — the tap fallback replaced its premise — so the anchor is never going to appear. It was the only reference to it in the repository; after this change there are none.

Change

One wrap_err_with, matching api::formula:

.wrap_err_with(|| format!("failed to fetch Homebrew cask '{requested_token}'"))

fetch_cask_url has three callers, and the shorter text reads correctly for each:

caller after
parent-tap probe (logged at debug!) brew-cask: x unavailable in parent tap metadata (failed to fetch Homebrew cask 'x': …)
tap path, as {api_err} published cask metadata was unavailable (failed to fetch Homebrew cask 'x': …) and mise could not evaluate Casks/x.rb
official cask, _ => return Err(api_err) failed to fetch Homebrew cask 'x': <http error>

The third one matters beyond tidiness. fetch_cask_url is shared, so a mistyped official token — brew-cask:firefx — was answered with advice about third-party taps. That is the defect #12645 set out to fix, and it fixes more cleanly by having the shared function say less rather than by scoping its explanation.

No test

Saying so rather than leaving it to be noticed:

  • Nothing pins this text. Searching src/ and e2e/ for most taps do not publish, will not proxy and failed to fetch Homebrew cask hits only the implementation line itself.
  • There is no HTTP test scaffolding under src/system/packages/brew/ — no mockito, no #[tokio::test]; cask/tests.rs is 7400 lines of synchronous tests over pure functions. Introducing the first one to assert a message string seemed disproportionate for text that is expected to keep changing.
  • The change removes wording; it adds no behavior.

I cannot build locally, so CI is the first execution.

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: 2.1.236.

Summary by CodeRabbit

  • Bug Fixes
    • Simplified the error message shown when fetching a package fails.
    • Fetching and validation behavior remain unchanged.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Team

Run ID: 3e343ed3-f30b-4c2f-9a07-27f168e1ed25

📥 Commits

Reviewing files that changed from the base of the PR and between e2925ed and accb8e3.

📒 Files selected for processing (1)
  • src/system/packages/brew/cask/fetch.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The Homebrew cask fetch failure message now provides concise generic context. Fetching and validation behavior remain unchanged.

Changes

Homebrew cask fetch errors

Layer / File(s) Summary
Simplify cask fetch error context
src/system/packages/brew/cask/fetch.rs
The fetch failure message now states that Homebrew cask metadata could not be fetched. Detailed third-party tap and installation guidance was removed.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 4185f

Homebrew cask metadata failures now show concise, accurate context without stale installation or tap guidance. Runtime fetch and validation behavior remain unchanged, with no current merge-readiness risk identified.

Suggested reviewers: jdx

Poem

A rabbit fetched a cask with care
And found a shorter message there
No tap advice, no lengthy trail
Just clear context when fetches fail
The brew path stays the same
With tidier words to name the flame

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing stale Homebrew advice from the brew cask metadata error.
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.

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.

❤️ Share

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

@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes stale, caller-inappropriate Homebrew tap guidance and a broken documentation link from the shared cask metadata fetch error.

  • Replaces the long remediation message with concise fetch context.
  • Leaves caller-specific handling and cask fetching behavior unchanged.
  • Aligns cask error composition with the formula implementation.

Confidence Score: 5/5

The PR appears safe to merge because it only corrects stale error text and preserves existing behavior.

No actionable defects or repository-rule violations remain in the reviewed change.

Important Files Changed

Filename Overview
src/system/packages/brew/cask/fetch.rs Shortens shared cask-fetch error context without changing fetching, validation, or fallback behavior.

Reviews (4): Last reviewed commit: "fix(brew-cask): drop stale brew advice f..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

This PR currently has failing checks. If this continues for 7 days, it will be closed automatically.

This is warning day 1 of 7.

Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it.

This comment was generated by an automated workflow.

@Marukome0743
Marukome0743 force-pushed the brew-cask-stale-tap-advice branch 2 times, most recently from 31eca33 to 75e7e92 Compare September 5, 2026 10:36
The message told the user mise would not proxy to the brew CLI and to
install with `brew`. That stopped being true when tap support gained a
Ruby fallback: fetch_cask now tries cask_from_ruby when the API metadata
is missing, and reports separately that the definition could not be
evaluated. The advice fired from inside that attempt, so it contradicted
what mise was doing.

It also linked brew.html#third-party-taps, which is not a section on that
page. It was the only reference to that anchor in the repository.

The formula path was already reorganized this way when the fallback
landed: the inner call carries a short context and the outer wrapper owns
the explanation. This gives the cask path the same shape. An official
cask that 404s now reports the fetch failure without tap advice that does
not apply to it.
@Marukome0743
Marukome0743 force-pushed the brew-cask-stale-tap-advice branch from 75e7e92 to 4185f1f Compare September 5, 2026 11:14
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