Bug | Fix rogue-tui auto update #143
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughVersion bumped from 0.3.0 to 0.3.1. The TUI installer was made version-aware: it detects an installed Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / Update Checker
participant Installer as RogueTuiInstaller
participant System as Local System (subprocess)
participant GitHub as GitHub Releases API
participant FS as Filesystem / pip
CLI->>Installer: install_rogue_tui()
Installer->>System: run `rogue-tui --version` (cached)
alt installed and up-to-date
System-->>Installer: version == target
Installer-->>CLI: log "already installed and up-to-date"
else installed but outdated
System-->>Installer: older version
Installer->>GitHub: GET /releases/{target_version} (or latest on fallback)
GitHub-->>Installer: release metadata + asset URL
Installer->>FS: download wheel, pip install
FS-->>Installer: install result
Installer-->>CLI: log "updated"
else not installed
System-->>Installer: not found / error
Installer->>GitHub: GET /releases/{target_version} (try latest if fails)
GitHub-->>Installer: release metadata
Installer->>FS: download wheel, pip install
FS-->>Installer: install result
Installer-->>CLI: log "installed"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rogue/common/tui_installer.py (1)
49-75: Consider explicit package name for clarity.The method correctly handles version-aware release fetching. However, line 53 calls
get_version()without arguments, relying on the default parameter. While this works, explicitly passingget_version("rogue-ai")would improve code clarity and make the dependency on the rogue-ai package version more obvious to future maintainers.🔎 Proposed change for explicitness
- version = get_version() + version = get_version("rogue-ai")
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rogue/common/tui_installer.pyrogue/common/version.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Useblackfor code formatting
Use type hints for all function signatures
Follow PEP 8 naming conventions (snake_case for variables and functions, PascalCase for classes)
Use try/except blocks for code that may raise exceptions
Useisortconventions for import organization
Files:
rogue/common/version.pyrogue/common/tui_installer.py
🧬 Code graph analysis (1)
rogue/common/tui_installer.py (1)
rogue/common/version.py (1)
get_version(8-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: rogue_sanity
- GitHub Check: codestyle
🔇 Additional comments (5)
rogue/common/version.py (1)
8-8: LGTM! Backward-compatible improvement.Adding a default parameter value for
package_nameis a good enhancement that maintains backward compatibility while simplifying common usage patterns. The default value "rogue-ai" aligns with the primary use case seen throughout the codebase.rogue/common/tui_installer.py (4)
5-5: LGTM! Imports are necessary and properly secured.The new imports support the version-aware installation logic:
refor version string parsingsubprocesswith appropriate security suppression for executingrogue-tui --versionget_versionfor determining target versionAlso applies to: 7-7, 18-18
198-207: LGTM! Version comparison is appropriate for exact matching.The method correctly determines whether to reinstall by comparing the installed TUI version against the current rogue-ai package version using string equality. This approach ensures the versions stay synchronized, which aligns with the PR objective of fixing auto-update behavior.
Note: The string comparison is intentional and suitable here, as the goal is exact version matching rather than semantic version range checking.
208-230: Breaking change confirmed: all callers have been properly updated.The method signature changed from
install_rogue_tui(self, upgrade: bool = False)toinstall_rogue_tui(self), removing theupgradeparameter. All three callers in the codebase (update_checker.pyline 220,__main__.pylines 217 and 262) have been updated to match the new signature without passing any arguments. The new implementation automatically determines whether to install or update based on version comparison, which is a cleaner approach aligned with the PR objective.
178-196: The regex pattern assumption is reasonable given the documented expected format and graceful error handling.The regex pattern
r"v?(\d+\.\d+\.\d+)"does assume a strict semver format, but this is explicitly documented in the code comment ("Parse output like 'rogue-tui v0.2.2'"). More importantly, parsing failures are handled gracefully—the method returnsNoneif the pattern doesn't match, which the caller (_should_reinstall_tui()) handles by returningTrueto trigger reinstallation. This is appropriate defensive programming for an external tool dependency and doesn't require changes.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rogue/common/tui_installer.py (1)
14-14: Update requests to 2.32.5 (latest as of Aug 2025).The code uses requests 2.32.4, but version 2.32.5 is available and recommended. While 2.32.4 already includes the CVE-2024-47081 fix, the newer patch should be adopted. Other dependencies (rich 14.0.0, loguru 0.7.3) are secure; platformdirs 4.3.8 could optionally be updated to 4.5.1.
🧹 Nitpick comments (1)
rogue/common/tui_installer.py (1)
211-269: Excellent implementation of version-aware installation with robust fallback logic.The updated flow provides a comprehensive solution for TUI version management:
Smart pre-checks (lines 216-233): Skip unnecessary downloads when already up-to-date, with clear messaging for each state (installed, updating, installing).
Fallback resilience (lines 235-252): If the version-specific download fails, automatically falling back to latest ensures users can still get a working TUI, which is a good UX decision.
Clear user communication: Rich console messages keep users informed throughout the process.
Proper error handling: Comprehensive try-except blocks with logging and user-friendly error messages.
Minor optimization: reduce repeated get_version() calls
Lines 225, 238, and 240 call
get_version()multiple times. Consider calling once at the start of the method and storing the result:def install_rogue_tui(self) -> bool: """Install rogue-tui from GitHub releases if not installed or needs update.""" console = Console() + current_version = get_version("rogue-ai") # Check if rogue-tui is already available if self._is_rogue_tui_installed(): # Check if version matches if not self._should_reinstall_tui(): console.print( "[green]✅ rogue-tui is already installed and up to date.[/green]", ) return True else: installed_version = self._get_installed_tui_version() - current_version = get_version("rogue-ai") console.print( f"[yellow]📦 Updating rogue-tui from " f"v{installed_version} to v{current_version}...[/yellow]", )And update lines 238 and 240:
- logger.exception(f"Failed to download rogue-tui for {get_version()}.") + logger.exception(f"Failed to download rogue-tui for {current_version}.") console.print( - f"[red]❌ Failed to download rogue-tui for {get_version()}.[/red]", + f"[red]❌ Failed to download rogue-tui for {current_version}.[/red]", )This is a minor optimization since
get_version()is likely fast, but it improves consistency and slightly reduces redundant calls.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rogue/common/tui_installer.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Useblackfor code formatting
Use type hints for all function signatures
Follow PEP 8 naming conventions (snake_case for variables and functions, PascalCase for classes)
Use try/except blocks for code that may raise exceptions
Useisortconventions for import organization
Files:
rogue/common/tui_installer.py
🧬 Code graph analysis (1)
rogue/common/tui_installer.py (1)
rogue/common/version.py (1)
get_version(8-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: rogue_sanity
- GitHub Check: codestyle
🔇 Additional comments (5)
rogue/common/tui_installer.py (5)
5-7: LGTM! Import additions support version detection.The new imports (
re,subprocess,get_version) are necessary for the version-aware installation logic. Thenosec: B404comment is appropriate since subprocess is used safely with fixed commands.Also applies to: 18-18
49-72: LGTM! Good refactor to make release fetching version-aware.The rename from
_get_latest_github_releaseto_get_release_from_github(version: str)improves flexibility by allowing retrieval of specific versions or "latest". The implementation correctly uses the version parameter in the URL and messaging.
93-106: LGTM! Version-aware download logic is well-implemented.The addition of
latest_version_overrideparameter enables flexible version targeting. The default behavior correctly downloads the version matchingget_version()(rogue-ai package version), with the ability to fall back to latest when needed.
201-209: LGTM! Version drift detection correctly compares TUI against rogue-ai package.The method appropriately determines whether reinstallation is needed by comparing the installed rogue-tui version with the rogue-ai package version. This design ensures the TUI stays synchronized with the main package, which aligns with the PR objective to fix version drift.
181-199: No actionable concern:@lru_cache(1)does not pose a stale result risk in this codebase.Every instantiation of
RogueTuiInstallerin the codebase creates a fresh instance (lines 217, 262 in__main__.pyand line 220 inupdate_checker.py). The caching only applies within the lifetime of a single instance, and instances are never reused across multiple operations. Within a single validation flow, the method may be called twice (lines 204 and 224), but both occur during pre-installation checks before any modification to the system. The cache optimization is appropriate and harmless.
Description
Motivation and Context
Type of Change
Changes Made
Screenshots/Examples (if applicable)
Checklist
uv run black .to format my codeuv run flake8 .and fixed all issuesuv run mypy --config-file .mypy.ini .and addressed type checking issuesuv run bandit -c .bandit.yaml -r .for security checksuv run pytestand all tests passTesting
Test Configuration:
Test Steps:
1.
2.
3.
Additional Notes
Related Issues/PRs