Skip to content

fix(cli): teach omni upgrade how to upgrade a Homebrew install - #4349

Open
PattaraS wants to merge 1 commit into
mainfrom
fix/brew-upgrade-command
Open

fix(cli): teach omni upgrade how to upgrade a Homebrew install#4349
PattaraS wants to merge 1 commit into
mainfrom
fix/brew-upgrade-command

Conversation

@PattaraS

@PattaraS PattaraS commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #2558

Summary

brew install omnigent-ai/tap/omnigent works today and the tap now tracks releases, but the follow-up ask on #2558 — upgrading a brewed install — still dead-ends:

$ omni upgrade --target-version 0.9.0
Error: No automatic upgrade command is known for this install.
       reinstall omnigent from your original source.

A Homebrew bottle records brew in its dist-info INSTALLER and ships no direct_url.json, so it reaches the registry branch of _build_upgrade_suggestion with an installer name that nothing matched, falling through to the non-runnable prose fallback. Brew users — the audience the tap exists for — had no supported upgrade path.

  • Map brewbrew upgrade omnigent-ai/tap/omnigent (runnable). The formula builds a virtualenv from pinned resources under the Cellar, so pip-ing into that keg would be undone by the next bump; the tap is the only correct source.
  • brew upgrade accepts neither a version pin nor extras, so --target-version and --extra return a non-runnable explanation rather than a command that would quietly ignore them.
  • --pre is a no-op: the tap carries only final releases.

Scoped to the upgrade path. The other two asks on #2558 (--HEAD, native /opt packaging) are untouched — see the note below.

Test Plan

Verified against a real Homebrew install on macOS 26.5.2 / arm64, not just unit tests:

brew tap omnigent-ai/tap
brew install omnigent-ai/tap/omnigent   # → 0.8.2, from the arm64_tahoe bottle

Confirmed the real install's metadata is the shape this branch keys on:

detected_installer: 'brew'
vcs_url: None

Feeding that exact metadata through the patched function:

command:  brew upgrade omnigent-ai/tap/omnigent
runnable: True

And the generated command is valid Homebrew:

$ brew upgrade --dry-run omnigent-ai/tap/omnigent
Warning: omnigent-ai/tap/omnigent 0.8.2 already installed   # exit 0

The test install was uninstalled and the tap untapped afterwards.

python -m pytest tests/cli/test_update_check.py    # 117 passed (5 new brew tests)
pre-commit run --files omnigent/update_check.py tests/cli/test_update_check.py
uvx pyrefly check omnigent/update_check.py         # 0 errors
.venv/bin/mypy omnigent/update_check.py            # Success

Demo

N/A — CLI behaviour, no visual change. Output quoted in the Test Plan.

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

Five unit tests: a brew row in the shared _build_upgrade_suggestion matrix (asserting runnable=True, since a prose fallback flipping to runnable would subprocess.run a literal sentence), plus dedicated tests for the --pre no-op and the two refusal paths.

Manual verification covers what unit tests can't: that a real bottle actually reports installer="brew" with no direct_url.json, and that brew upgrade <tap formula> is a command Homebrew accepts. Both were run on a live install as quoted above. The one step not exercised end-to-end is omni upgrade executing the upgrade — that needs a brewed copy genuinely behind a newer tap version, which doesn't exist while the tap is current; the command itself was validated with --dry-run.

Note for reviewers

#2558 also asks for a working --HEAD formula and native /opt-respecting packaging. Both are out of scope here, and worth knowing where they'd live: the tap's Formula/omnigent.rb is generated from .github/scripts/homebrew/omnigent.rb.template in this repo (I diffed them — the install/test blocks are byte-identical), so a head stanza committed to the tap would be overwritten on the next release bump. It has to go in the template. --HEAD is also more than a stanza: the template's install pip-installs release-pinned resources, which don't describe a build of a moving main. Since the closing keyword above resolves the whole issue, it may be worth opening a narrower follow-up for those two.

Changelog

omni upgrade now upgrades Homebrew installs via brew upgrade instead of reporting that no upgrade command is known.

This pull request and its description were written by Isaac.

A Homebrew bottle records `brew` in its dist-info `INSTALLER` and ships no
`direct_url.json`, so it reaches the registry branch of
`_build_upgrade_suggestion` with an installer name nothing matched. It fell
through to the unknown-installer prose fallback and `omni upgrade` dead-ended:

    Error: No automatic upgrade command is known for this install.
           reinstall omnigent from your original source.

That left brew users — the audience the tap exists for — with no supported way
forward, even though `brew install` itself now works and the tap tracks
releases.

Map `brew` to `brew upgrade omnigent-ai/tap/omnigent`. Since the formula builds
a virtualenv from pinned resources under the Cellar, pip-ing into that keg would
be undone by the next bump, so the tap is the only correct upgrade source.

`brew upgrade` accepts neither a version pin nor extras, so `--target-version`
and `--extra` return a non-runnable explanation instead of a command that would
quietly ignore them. `--pre` is a no-op: the tap carries only final releases.

Co-authored-by: Isaac
@github-actions github-actions Bot added size/M Pull request size: M P1-high Priority: major feature broken, no workaround labels Aug 7, 2026
@omnigent-ci

omnigent-ci Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Review

Blocking issues

None. The change is correct and well-scoped.

  • The brew branch is inserted on the registry path (info.vcs_url is falsy), which matches how a Homebrew bottle actually presents: installer="brew" and no direct_url.json. This is exactly the case that previously fell through to the non-runnable "reinstall omnigent from your original source" prose.
  • The branch returns before registry_spec is computed, so it fully owns the brew case and can't accidentally fall into the pip/uv logic below.
  • _PRERELEASE_FLAG has no "brew" key, so pre is "" and --pre is correctly a no-op — the returned command is the bare brew upgrade line, matching the test.
  • The two refusal paths (target_version, extras) return runnable=False, so _run_upgrade_command never shlex.splits the prose sentence into a subprocess. This is the important safety property, and the runnable=True matrix row plus the dedicated refusal tests lock it in.

Security vulnerabilities

None. _BREW_TAP_FORMULA is a hard-coded constant and the runnable command contains no interpolated user input, so there's no command-injection surface via shlex.split. No new dependencies, extras, or lockfile changes.

Non-blocking notes

  • extras refusal only triggers when extras are present in metadata or passed via --extra; a brew bottle records no extras, so in practice this path is reached only through --extra. That's the intended behavior — just noting the branch is effectively --extra-only for brew installs.
  • The refusal messages embed the runnable form (`brew upgrade omnigent-ai/tap/omnigent`) inside prose, which is a nice touch — the user still learns the correct command even when their pin/extras can't be honored.
  • Docstring for _build_upgrade_suggestion was updated to include brew; the _UpgradeSuggestion.runnable docstring was also correctly broadened to cover the "request the installer cannot honor" case. Good doc hygiene.

Summary

A tight, correct bug fix. It closes a real dead-end for Homebrew users by mapping brew to a runnable brew upgrade <tap> command while deliberately refusing (non-runnable) the version-pin and extras requests Homebrew can't satisfy, so the interactive "run this now?" prompt is never offered a literal sentence. Tests cover the runnable case, the --pre no-op, and both refusal paths, and the author's manual verification confirms the real bottle metadata shape the branch keys on. No blocking or security concerns; the out-of-scope --HEAD/native-packaging asks are appropriately deferred with a clear pointer to the generated-formula template. Ready to merge.


Automated review by Polly · workflow run

@PattaraS
PattaraS requested a review from fanzeyi August 7, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1-high Priority: major feature broken, no workaround size/M Pull request size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Distribution / Installation / Enterprise Readiness

1 participant