fix: stabilisation P0–P5 — Homebrew cmd, progress config, CLI drift, exception narrowing#117
Open
fix: stabilisation P0–P5 — Homebrew cmd, progress config, CLI drift, exception narrowing#117
Conversation
…exception narrowing
P0 (homebrew.py):
- Replace shell-substitution command in get_all_homebrew_casks() with
`brew info --json=v2 --eval-all --cask` via run_command_secure() (argv list).
The old command used $(...) which shlex.split passes literally when shell=False.
- Fix is_homebrew_available() to use get_brew_command() (configured path)
instead of bare "brew --version".
- Fix get_homebrew_path() except block: re-raise HomebrewError before the
broad OSError fallback so it propagates correctly.
- Update tests to assert exact argv shape, not just that mock was called.
P1 (setup_handlers.py, outdated_handlers.py):
- setup_handlers: replace _config["ui"]["show_progress"] mutation with
config.set("no_progress", True) — the old write targeted a dead key;
Config.show_progress reads _config["no_progress"], not _config["ui"][...].
- setup_handlers: replace remaining _config direct mutations with config.set()
for no_color and adaptive_rate_limiting.
- outdated_handlers: remove config.set("show_progress", False) — redundant
since show_progress is derived from no_progress, not stored independently.
- Update tests to assert config.set() calls instead of _config state.
P2 (cli.py):
- Add --output-file / dest=output_file to the Export Options group.
brew_handlers.py accessed options.output_file unconditionally but the
option had no parser backing, causing AttributeError on real CLI invocations
when --export was used.
P4 (homebrew.py, apps/finder.py, outdated_handlers.py):
- Narrow except Exception in get_homebrew_path (HomebrewError + OSError).
- Narrow async availability check in finder.py (AttributeError + RuntimeError).
- Narrow get_applications parsing error (KeyError + IndexError + TypeError).
- Narrow filter_out_brews fallback (ValueError + TypeError + AttributeError).
- Top-level and setup handler catches remain broad — justified last resort.
P5 (README.md, TODO.md):
- Replace "Production-Ready" badge/heading with "Beta — Stabilising".
- Update test count (1,885 → 2,173) and coverage claim (61% → 78%).
- Rewrite Overview paragraph to reference stabilisation roadmap.
- Rewrite TODO.md Active Work section with P0-P5 issue definitions,
acceptance criteria, and verify commands from PROJECT_REVIEW.md.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Reviewer's GuideStabilises Homebrew integration, progress configuration, and CLI export options while narrowing exception handling and aligning documentation with the current beta/stabilisation cycle. Sequence diagram for fetching all Homebrew casks via run_command_securesequenceDiagram
participant Caller
participant homebrew_module as homebrew
participant config_module as config
participant utils_module as utils
participant Homebrew
Caller->>homebrew_module: get_all_homebrew_casks()
activate homebrew_module
homebrew_module->>config_module: get_brew_command()
activate config_module
config_module-->>homebrew_module: brew_path
deactivate config_module
homebrew_module->>utils_module: run_command_secure([brew_path, info, --json=v2, --eval-all, --cask], timeout=120)
activate utils_module
utils_module->>Homebrew: exec brew info --json=v2 --eval-all --cask
Homebrew-->>utils_module: stdout, returncode
utils_module-->>homebrew_module: stdout, returncode
deactivate utils_module
alt returncode == 0
homebrew_module->>homebrew_module: parse JSON stdout
homebrew_module-->>Caller: list of cask dicts
else returncode != 0
homebrew_module->>homebrew_module: build error message
homebrew_module-->>Caller: raise HomebrewError
end
deactivate homebrew_module
Sequence diagram for CLI export with --output-file optionsequenceDiagram
actor User
participant cli_module as cli
participant argparse
participant handler_module as brew_handlers
participant filesystem as FileSystem
User->>cli_module: versiontracker --export json --output-file results.json
activate cli_module
cli_module->>argparse: get_arguments()
activate argparse
argparse-->>cli_module: options(export=json, output_file=results.json)
deactivate argparse
cli_module->>handler_module: handle_export(options)
activate handler_module
handler_module->>handler_module: generate_export_data(format=json)
handler_module-->>handler_module: data
alt options.output_file is set
handler_module->>FileSystem: open results.json for write
FileSystem-->>handler_module: file handle
handler_module->>FileSystem: write data
FileSystem-->>handler_module: close
else
handler_module-->>User: write data to stdout
end
handler_module-->>cli_module: exit_code
deactivate handler_module
cli_module-->>User: process exit
deactivate cli_module
Flow diagram for canonicalising progress configuration flagsflowchart TD
A["CLI options"] --> B{"options.no_progress is True?"}
B -- "Yes" --> C["setup_handlers.handle_configure_from_options"]
C --> D["current_config.set('no_progress', True)"]
B -- "No" --> E["No change to no_progress"]
subgraph Outdated_handlers
F["_update_config_from_options"] --> G["config.set('no_progress', True)"]
end
D --> H["Config._config['no_progress'] = True"]
G --> H
H --> I["Config.show_progress computed as not no_progress"]
I --> J["UI components and progress bars respect show_progress"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
🔒 Security Analysis ReportSecurity Analysis ReportGenerated: Mon Mar 30 05:50:41 UTC 2026 Bandit Security ScanSafety Check ResultsPip-Audit Results |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e03c6f2c28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -54,7 +54,6 @@ def _update_config_from_options(options: Any) -> None: | |||
| config = get_config() | |||
| if hasattr(config, "set"): | |||
| config.set("no_progress", True) | |||
There was a problem hiding this comment.
--no-progress effective for outdated checks
_update_config_from_options now only sets no_progress, but the outdated batch path still reads config._config["ui"]["show_progress"] in versiontracker/version/batch.py (lines 61-63). Because this key is no longer updated here, --check-outdated --no-progress will still render progress bars (default True), which regresses CLI behavior and breaks non-interactive/scripted usage that relies on quiet output.
Useful? React with 👍 / 👎.
Comment on lines
+51
to
54
| except HomebrewError: | ||
| return False | ||
| except OSError as e: | ||
| logging.warning("Homebrew availability check failed: %s", e) |
There was a problem hiding this comment.
is_homebrew_available() switched to run_command_secure, which raises versiontracker.exceptions.TimeoutError when brew --version exceeds the timeout, but this function now only catches HomebrewError and OSError. In that timeout case it raises instead of returning False, so callers that treat this as a boolean guard (for example async/sync Homebrew fallback checks) can fail hard instead of degrading gracefully.
Useful? React with 👍 / 👎.
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.
Summary
get_all_homebrew_casks(): replaces shell-substitution$(ls $(brew --repository)/...)(whichshlex.splitpasses literally whenshell=False) withbrew info --json=v2 --eval-all --caskviarun_command_secure(). Also fixesis_homebrew_available()to use the configured brew path instead of barebrew. Tests now assert the exact argv shape.--no-progress:setup_handlers.pywas writing to_config["ui"]["show_progress"]— a dead key;Config.show_progressis derived from_config["no_progress"]. Changed toconfig.set("no_progress", True)everywhere. Removed the redundantconfig.set("show_progress", False)call inoutdated_handlers.py.--output-fileto the CLI export group:brew_handlers.pyaccessedoptions.output_fileunconditionally when--exportwas set, but the option had no parser backing (would raiseAttributeErroron real CLI use).except Exceptionin touched modules:get_homebrew_path, async availability check infinder.py, app data parsing infinder.py, and thefilter_out_brewsfallback inoutdated_handlers.py. Top-level and setup-handler catches remain broad (justified last-resort guards).PROJECT_REVIEW.md.Test plan
pytest -q tests/test_homebrew_advanced.py tests/test_homebrew.py— P0 command shape assertionspytest -q tests/handlers/test_setup_handlers.py tests/test_outdated_handlers.py tests/test_cli.py— P1/P2 progress config and CLIpytest -q— full suite (2,158 passed, 15 skipped locally)🤖 Generated with Claude Code
Summary by Sourcery
Stabilise Homebrew integration, CLI configuration flags, and documentation to align the project with its current beta state.
New Features:
Bug Fixes:
Enhancements:
Documentation: