Self update version mgmt #199
Conversation
|
Hi @dhvll! This PR has merge conflicts with the latest main branch. Could you please rebase your branch to resolve them? git fetch origin
git rebase origin/main
# resolve conflicts
git push --force-with-leaseOnce rebased, we'll merge this right away - the implementation looks great! |
|
Thanks for this work! Please rebase onto main to resolve conflicts: |
WalkthroughAdds a self-update system: dynamic package/version utilities, manifest schema and compatibility checks, an UpdateService to check/download/verify/install updates with rollback and persisted state, CLI commands for update/channel management, new tests, README expansions, and two new Python dependencies. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CortexCLI
participant Updater as UpdateService
participant Manifest as UpdateManifest
participant Network as Remote (manifest/asset)
participant PkgMgr as pip
User->>CLI: run command (install / update / channel)
CLI->>Updater: check_for_updates(force?/channel?)
Updater->>Network: fetch manifest JSON
Network-->>Updater: manifest JSON
Updater->>Manifest: parse & filter (channel, system)
Manifest-->>Updater: release candidate(s)
Updater-->>CLI: notify update available / none
alt User triggers update
User->>CLI: cortex update
CLI->>Updater: perform_update(dry_run?/force?)
Updater->>Network: download release artifact
Network-->>Updater: artifact + sha256
Updater->>Updater: verify checksum
Updater->>PkgMgr: pip install artifact (or simulate if dry-run)
PkgMgr-->>Updater: install result
Updater->>Updater: persist state / write logs
alt install failure
Updater->>Updater: rollback to previous version
end
Updater-->>CLI: result + release notes
CLI->>User: display outcome
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
* Add PR automation (CODEOWNERS + template) * Remove duplicate automation workflow --------- Co-authored-by: Mike Morgan <allbots@allbots.io>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cortex/__init__.py (1)
1-14: Duplicate assignments override dynamic version logic.The file has two
__version__assignments and two__all__assignments:
- Lines 3-6 dynamically resolve version from metadata (good)
- Line 12 overwrites it with static
"0.1.0"(defeats the purpose)- Line 8 sets
__all__ = ["__version__"]- Line 14 overwrites
__all__without including"__version__"This appears to be a merge artifact or incomplete refactoring. The dynamic version logic at lines 3-6 is rendered useless.
from importlib import metadata try: __version__ = metadata.version("cortex-linux") except metadata.PackageNotFoundError: __version__ = "0.1.0" -__all__ = ["__version__"] from .cli import main from .packages import PackageManager, PackageManagerType -__version__ = "0.1.0" - -__all__ = ["main", "PackageManager", "PackageManagerType"] +__all__ = ["__version__", "main", "PackageManager", "PackageManagerType"]
🧹 Nitpick comments (5)
test/test_update_service.py (1)
103-104: Use_for the unusedselfparameter to satisfy linter.The lambda replaces an instance method but doesn't use
self. Convention is to use_for intentionally unused parameters.- monkeypatch.setattr(UpdateService, "_fetch_manifest", lambda self: manifest) + monkeypatch.setattr(UpdateService, "_fetch_manifest", lambda _: manifest)cortex/versioning.py (1)
53-58: Consider narrowing the exception catch.The bare
Exceptioncatch works but is overly broad. Since this fallback handles import/attribute access, narrowing to specific exceptions would be more precise without losing safety.try: from cortex import __version__ as package_version # type: ignore raw_version = package_version - except Exception: + except (ImportError, AttributeError): raw_version = "0.0.0"cortex/cli.py (1)
195-239: Consider extracting release notes display to reduce duplication.The release notes display logic (lines 225-228 and 234-237) is duplicated. This also contributes to the cognitive complexity flagged by SonarCloud. Consider extracting a helper method.
+ def _print_release_notes(self, release): + if release.release_notes: + self._print_status("🆕", "What's new:") + for line in release.release_notes.strip().splitlines(): + print(f" {line}")Then replace both occurrences with
self._print_release_notes(release).cortex/updater.py (1)
270-271: Minor: Use list unpacking for cleaner syntax.- cmd = [sys.executable, "-m", "pip"] + args + cmd = [sys.executable, "-m", "pip", *args]cortex/update_manifest.py (1)
129-143: Consider implementing manifest signature verification.The
signaturefield is parsed but never validated. For a complete security model, the manifest should be cryptographically signed and verified before trusting its contents. The current implementation relies solely on HTTPS transport security.Would you like me to help design a signature verification approach, or should this be tracked as a follow-up issue?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
LLM/requirements.txt(1 hunks)README.md(1 hunks)README_DEPENDENCIES (1).md(0 hunks)cortex/__init__.py(1 hunks)cortex/cli.py(6 hunks)cortex/update_manifest.py(1 hunks)cortex/updater.py(1 hunks)cortex/versioning.py(1 hunks)test/test_update_service.py(1 hunks)
💤 Files with no reviewable changes (1)
- README_DEPENDENCIES (1).md
🧰 Additional context used
🧬 Code graph analysis (4)
cortex/cli.py (3)
cortex/update_manifest.py (2)
UpdateChannel(18-28)from_string(23-28)cortex/updater.py (8)
ChecksumMismatch(59-60)InstallError(63-64)UpdateError(55-56)UpdateService(67-296)get_channel(100-106)perform_update(175-231)check_for_updates(130-172)set_channel(108-111)cortex/versioning.py (1)
from_string(30-35)
test/test_update_service.py (3)
cortex/update_manifest.py (5)
UpdateChannel(18-28)UpdateManifest(130-177)SystemInfo(32-45)current(39-45)find_latest(158-177)cortex/versioning.py (1)
CortexVersion(23-38)cortex/updater.py (3)
UpdateService(67-296)get_channel(100-106)perform_update(175-231)
cortex/versioning.py (1)
cortex/update_manifest.py (2)
from_string(23-28)current(39-45)
cortex/update_manifest.py (1)
cortex/versioning.py (3)
CortexVersion(23-38)is_newer_version(63-66)from_string(30-35)
🪛 GitHub Actions: Cortex Automation
test/test_update_service.py
[error] 1-1: Test collection failed due to import error: missing 'requests' dependency in environment, causing several test modules to fail during collection.
cortex/updater.py
[error] 19-19: ModuleNotFoundError: No module named 'requests'
🪛 GitHub Check: SonarCloud Code Analysis
cortex/cli.py
[failure] 195-195: Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
🪛 Ruff (0.14.6)
cortex/cli.py
213-213: Do not catch blind exception: Exception
(BLE001)
218-218: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
229-229: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
247-247: Do not catch blind exception: Exception
(BLE001)
260-260: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
271-271: Possible SQL injection vector through string-based query construction
(S608)
test/test_update_service.py
104-104: Unused lambda argument: self
(ARG005)
cortex/versioning.py
34-34: Avoid specifying long messages outside the exception class
(TRY003)
57-57: Do not catch blind exception: Exception
(BLE001)
cortex/updater.py
90-90: Do not catch blind exception: Exception
(BLE001)
257-259: Avoid specifying long messages outside the exception class
(TRY003)
271-271: Consider [sys.executable, "-m", "pip", *args] instead of concatenation
Replace with [sys.executable, "-m", "pip", *args]
(RUF005)
274-274: subprocess call: check for execution of untrusted input
(S603)
283-283: Avoid specifying long messages outside the exception class
(TRY003)
cortex/update_manifest.py
28-28: Avoid specifying long messages outside the exception class
(TRY003)
53-53: Do not catch blind exception: Exception
(BLE001)
72-72: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (22)
test/test_update_service.py (5)
10-30: LGTM!The
make_manifesthelper is well-structured with sensible defaults for testing purposes.
33-39: LGTM!Good practice to use a fixed
SystemInfofor deterministic test behavior.
42-65: LGTM!Good coverage of the version selection logic, ensuring the highest eligible version is selected.
68-87: LGTM!Good coverage of channel persistence, verifying both the API and the underlying file state.
90-111: LGTM!Good test for dry-run behavior — confirms the release is identified but no update is applied.
cortex/versioning.py (2)
22-38: LGTM!Clean dataclass design with immutability (
frozen=True) and a well-structured factory method for validation.
63-66: LGTM!Simple and correct version comparison using the parsed
Versionobjects.LLM/requirements.txt (1)
3-4: Unfortunately, I'm unable to access the repository to verify the review comment. The repository clone operation failed, which prevents me from:
- Confirming whether
cortex/updater.pyandcortex/versioning.pyactually use therequestsandpackagingdependencies- Checking the repository's dependency structure (root
requirements.txt,setup.py,pyproject.toml)- Determining whether
LLM/requirements.txtis separate from or integrated with root dependencies- Verifying the test configuration and why the pipeline might be failing
Without access to the codebase, I cannot definitively verify whether the review comment's claims are accurate or if the dependencies are indeed in the wrong location.
Dependencies may be added to wrong requirements file — needs verification.
The review identifies that
requestsandpackagingdependencies were added toLLM/requirements.txtbut claims they're needed bycortex/updater.pyandcortex/versioning.py. However, without repository access, I cannot confirm:
- Whether these modules actually import these dependencies
- The repository's dependency structure (root vs. subdirectory requirements)
- Whether the pipeline failure is caused by missing dependencies in the root scope
A maintainer should verify if these dependencies need to be in the root
requirements.txtorsetup.pyinstead of (or in addition to)LLM/requirements.txt.cortex/cli.py (3)
241-256: LGTM!The broad exception catch at line 247 is appropriate here since update notifications are non-critical and should never disrupt the primary install workflow. Silently failing is the correct behavior.
258-272: LGTM!Channel management methods are well-structured with proper validation and error handling.
606-616: LGTM!The CLI argument parsing for
updateandchannelcommands is well-structured, with proper choices derived from theUpdateChannelenum.cortex/updater.py (6)
93-97: LGTM: Atomic state file writes.The write-to-temp-then-replace pattern prevents data corruption if the process is interrupted during write.
130-172: LGTM!The update check logic is well-structured with appropriate caching to avoid excessive network calls, and properly handles timezone-aware datetimes.
209-231: LGTM: Robust update workflow with rollback.Good implementation with proper cleanup in
finallyblock and automatic rollback on failure before re-raising the exception.
234-248: LGTM!Streaming download with chunked writes and checksum verification provides both memory efficiency and security.
266-268: Rollback depends on PyPI availability.The rollback reinstalls from PyPI using
{PACKAGE_NAME}=={previous.raw}. If the previous version is yanked or unavailable, the rollback will fail. Consider documenting this limitation or caching the previous wheel before upgrade.
299-319: LGTM!Proper serialization helper for caching release data to state file.
cortex/update_manifest.py (5)
18-28: LGTM!Clean enum with case-insensitive parsing and helpful error message listing valid options.
48-54: LGTM!Lazy import with graceful fallback handles the optional
distropackage correctly.
81-94: LGTM!Compatibility checking logic is correct: empty constraint lists match any system, while non-empty lists require membership.
107-127: LGTM!
ReleaseEntry.from_dictproperly parses nested structures, andis_compatiblecorrectly implements OR semantics across compatibility rules.
158-177: LGTM!
find_latestcorrectly filters for compatible releases newer than the current version and returns the highest version.
| from pathlib import Path | ||
| from typing import Any, Dict, Optional, Tuple | ||
|
|
||
| import requests |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if requests is declared in any dependency file
echo "=== Checking setup.py ==="
if [ -f setup.py ]; then
cat setup.py
else
echo "setup.py not found"
fi
echo ""
echo "=== Checking requirements*.txt ==="
fd -t f 'requirements.*\.txt' 2>/dev/null && echo "Found requirements files:" && fd -t f 'requirements.*\.txt' --exec cat {} \;
echo ""
echo "=== Checking pyproject.toml ==="
if [ -f pyproject.toml ]; then
cat pyproject.toml
else
echo "pyproject.toml not found"
fi
echo ""
echo "=== Checking setup.cfg ==="
if [ -f setup.cfg ]; then
cat setup.cfg
else
echo "setup.cfg not found"
fiRepository: cortexlinux/cortex
Length of output: 180
It appears the repository clone failed. However, I can work with the information already provided in your review comment. Let me analyze what we have:
From the original review comment, I can see:
- File: cortex/updater.py
- Line 19:
import requests - Issue: Pipeline failed with
ModuleNotFoundError: No module named 'requests' - Context: The file is new and implements UpdateService
The review comment appropriately flags this as a critical issue since:
- The import exists in the code
- The pipeline failed due to missing the module
- The suggestion to add it to dependencies (setup.py, requirements.txt, or pyproject.toml) is correct
Given that:
- This is a new file being added in the PR
- The import is present
- The pipeline actually failed with this error
- The suggested fix (adding to dependencies) is the correct resolution
The review comment is accurate and addresses a legitimate critical issue. The rewritten version should maintain this severity while being concise.
Add requests to project dependencies to resolve pipeline ModuleNotFoundError.
The pipeline failed with ModuleNotFoundError: No module named 'requests'. Add requests to the project's dependency declarations (e.g., requirements.txt, setup.py, or pyproject.toml).
🧰 Tools
🪛 GitHub Actions: Cortex Automation
[error] 19-19: ModuleNotFoundError: No module named 'requests'
🤖 Prompt for AI Agents
In cortex/updater.py around line 19, the code imports the third-party package
"requests" which caused a pipeline ModuleNotFoundError; to fix, add "requests"
to the project's declared dependencies (e.g., add to requirements.txt, include
in install_requires in setup.py, or add to pyproject.toml/poetry dependencies)
and ensure CI installs dependencies so the module is available during pipeline
runs.
…rtexlinux#208) Comprehensive code review and improvement of the Cortex Linux repository. - Added command validation in coordinator.py to prevent shell injection - Expanded dangerous command patterns in sandbox_executor.py (20+ new patterns) - Created cortex/utils/commands.py with secure command execution utilities - Created ASSESSMENT.md with full code audit report - Created ROADMAP.md with prioritized improvement plan - Rewrote README.md with comprehensive documentation - Updated CONTRIBUTING.md with detailed guidelines - Created CHANGELOG.md following Keep a Changelog format - Fixed automation.yml (wrong test directory tests/ → test/) - Added Python version matrix (3.10, 3.11, 3.12) - Added lint job (black, pylint) - Added security job (bandit, safety) - Added coverage reporting with Codecov - Created root requirements.txt with core dependencies - Created requirements-dev.txt with dev dependencies - Updated setup.py to use root requirements.txt - Standardized Python version to >=3.10 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Mike Morgan <allbots@allbots.io> Co-authored-by: Claude <noreply@anthropic.com>
Implements comprehensive user preferences system with YAML storage, validation, import/export. 38 tests passing.
Implements config export/import with APT/PIP/NPM detection, validation, diff command. 32 tests, 87% coverage.
dhvll
force-pushed
the
self-update-version-mgmt
branch
from
923ccc0 to
aaca54e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
496-902: Remove massive content duplication.Lines 496-902 duplicate the entire Features, Quick Start, Installation, Usage, Configuration, Architecture, Development, Contributing, Roadmap, FAQ, and Community sections that already appear at lines 73-495. This nearly doubles the README file size unnecessarily.
Remove lines 496-902 entirely, keeping only the content at lines 1-495.
♻️ Duplicate comments (2)
cortex/updater.py (1)
19-19: Verifyrequestslibrary is in dependencies.Past review comments flagged that
requestsis imported but may not be declared in project dependencies, causing pipeline failures. Ensure it's added torequirements.txt,setup.py, orpyproject.toml.#!/bin/bash # Check if requests is declared in dependency files echo "=== Checking for 'requests' in dependencies ===" fd -t f 'requirements.*\.txt$|setup\.py$|pyproject\.toml$' --exec grep -l "requests" {}README.md (1)
423-438: Fix broken HTML structure in FAQ.The
<details>block for "What operating systems are supported?" (line 423) is never closed, and the "Keeping Cortex Up to Date" section (lines 426-435) is incorrectly placed inside this FAQ entry. The FAQ answer appears after the update section at line 437.Based on past review comments, this issue was marked as addressed but is still present. Apply this fix:
<details> <summary><strong>What operating systems are supported?</strong></summary> -## Keeping Cortex Up to Date - -- Cortex automatically checks for new releases (stable by default) when you run `cortex install ...`. Disable with `CORTEX_UPDATE_CHECK=0`. -- See the current channel or switch tracks: - - `cortex channel show` - - `cortex channel set beta` -- Upgrade in-place with release notes, checksum verification, and automatic rollback on failure: - - `cortex update` (use `--dry-run` to preview, `--channel beta` to override per run). -- Update metadata lives in `~/.config/cortex/updater/`, including logs and last upgrade state. - -### Join the Community Currently Ubuntu 24.04 LTS. Other Debian-based distributions coming soon. </details> + +## Keeping Cortex Up to Date + +- Cortex automatically checks for new releases (stable by default) when you run `cortex install ...`. Disable with `CORTEX_UPDATE_CHECK=0`. +- See the current channel or switch tracks: + - `cortex channel show` + - `cortex channel set beta` +- Upgrade in-place with release notes, checksum verification, and automatic rollback on failure: + - `cortex update` (use `--dry-run` to preview, `--channel beta` to override per run). +- Update metadata lives in `~/.config/cortex/updater/`, including logs and last upgrade state.
🧹 Nitpick comments (3)
cortex/__init__.py (1)
11-11: Optional: Sort__all__for consistency.Static analysis suggests sorting
__all__alphabetically for better maintainability.-__all__ = ["__version__", "main", "PackageManager", "PackageManagerType"] +__all__ = ["PackageManager", "PackageManagerType", "__version__", "main"]cortex/cli.py (1)
195-239: Consider refactoring to reduce cognitive complexity.The
updatemethod has a cognitive complexity of 18, slightly above the recommended threshold of 15. While the logic is clear, extracting release display logic into a helper method would improve maintainability.Consider extracting the release notes display logic (lines 225-228, 234-237) into a helper method:
def _display_release_notes(self, release: ReleaseEntry): if release.release_notes: self._print_status("🆕", "What's new:") for line in release.release_notes.strip().splitlines(): print(f" {line}")This would reduce complexity and enable reuse across update flows.
cortex/updater.py (1)
270-271: Optional: Use unpacking for list concatenation.Static analysis suggests using unpacking instead of concatenation for cleaner code.
- cmd = [sys.executable, "-m", "pip"] + args + cmd = [sys.executable, "-m", "pip", *args]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
LLM/requirements.txt(1 hunks)README.md(1 hunks)README_DEPENDENCIES (1).md(0 hunks)cortex/__init__.py(1 hunks)cortex/cli.py(6 hunks)cortex/update_manifest.py(1 hunks)cortex/updater.py(1 hunks)cortex/versioning.py(1 hunks)test/test_update_service.py(1 hunks)
💤 Files with no reviewable changes (1)
- README_DEPENDENCIES (1).md
🚧 Files skipped from review as they are similar to previous changes (1)
- LLM/requirements.txt
🧰 Additional context used
🧬 Code graph analysis (5)
cortex/updater.py (2)
cortex/update_manifest.py (10)
ReleaseEntry(98-126)SystemInfo(32-45)UpdateChannel(18-28)UpdateManifest(130-177)current(39-45)from_string(23-28)from_dict(65-79)from_dict(108-120)from_dict(136-143)find_latest(158-177)cortex/versioning.py (3)
CortexVersion(23-38)get_installed_version(41-60)from_string(30-35)
cortex/cli.py (3)
cortex/update_manifest.py (2)
UpdateChannel(18-28)from_string(23-28)cortex/updater.py (8)
ChecksumMismatch(59-60)InstallError(63-64)UpdateError(55-56)UpdateService(67-296)get_channel(100-106)perform_update(175-231)check_for_updates(130-172)set_channel(108-111)cortex/versioning.py (1)
from_string(30-35)
cortex/__init__.py (1)
cortex/packages.py (2)
PackageManager(23-452)PackageManagerType(16-20)
cortex/update_manifest.py (1)
cortex/versioning.py (3)
CortexVersion(23-38)is_newer_version(63-66)from_string(30-35)
test/test_update_service.py (2)
cortex/update_manifest.py (5)
UpdateChannel(18-28)UpdateManifest(130-177)SystemInfo(32-45)current(39-45)find_latest(158-177)cortex/updater.py (3)
UpdateService(67-296)get_channel(100-106)perform_update(175-231)
🪛 GitHub Check: SonarCloud Code Analysis
cortex/cli.py
[failure] 195-195: Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
🪛 LanguageTool
README.md
[style] ~372-~372: Consider using a more formal and expressive alternative to ‘amazing’.
Context: ...g-feature) 3. **Commit** your changes (git commit -m 'Add amazing feature') 4. **Push** to the branch (...
(AWESOME)
🪛 markdownlint-cli2 (0.18.1)
README.md
246-246: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
333-333: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.6)
cortex/updater.py
90-90: Do not catch blind exception: Exception
(BLE001)
257-259: Avoid specifying long messages outside the exception class
(TRY003)
271-271: Consider [sys.executable, "-m", "pip", *args] instead of concatenation
Replace with [sys.executable, "-m", "pip", *args]
(RUF005)
274-274: subprocess call: check for execution of untrusted input
(S603)
283-283: Avoid specifying long messages outside the exception class
(TRY003)
cortex/versioning.py
34-34: Avoid specifying long messages outside the exception class
(TRY003)
57-57: Do not catch blind exception: Exception
(BLE001)
cortex/cli.py
213-213: Do not catch blind exception: Exception
(BLE001)
218-218: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
229-229: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
247-247: Do not catch blind exception: Exception
(BLE001)
260-260: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
271-271: Possible SQL injection vector through string-based query construction
(S608)
cortex/__init__.py
11-11: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
cortex/update_manifest.py
28-28: Avoid specifying long messages outside the exception class
(TRY003)
53-53: Do not catch blind exception: Exception
(BLE001)
72-72: Avoid specifying long messages outside the exception class
(TRY003)
test/test_update_service.py
104-104: Unused lambda argument: self
(ARG005)
🔇 Additional comments (37)
test/test_update_service.py (6)
10-31: LGTM!The
make_manifesthelper provides a clean, reusable way to construct test manifests with sensible defaults.
33-40: LGTM!The
current_systemhelper provides consistent test fixtures for system information.
42-66: LGTM!The test correctly verifies that the manifest selection logic chooses the newest compatible release (0.2.0) over an older one (0.1.5).
68-88: LGTM!The test validates channel persistence across set operations and confirms the state is correctly serialized to JSON.
90-112: Remove unused lambda parameter.Line 104: The lambda function includes an unused
selfparameter. Sinceget_installed_versionis a module-level function (not a method), the monkeypatch doesn't need theselfparameter.- monkeypatch.setattr("cortex.updater.get_installed_version", lambda: CortexVersion.from_string("0.1.0")) + monkeypatch.setattr("cortex.updater.get_installed_version", lambda: CortexVersion.from_string("0.1.0"))Actually, the current code is correct. The static analysis warning is a false positive—the lambda doesn't have a
selfparameter. Ignore the hint.
90-112: LGTM!The test correctly validates dry-run behavior: the release is identified, no update is applied, and the result message indicates dry run mode.
cortex/cli.py (7)
13-14: LGTM!Clean imports of the update subsystem components.
28-32: LGTM!UpdateService initialization is properly placed in the constructor.
67-68: LGTM!Good user experience: checking for updates before install operations.
241-256: LGTM!Silent failure on update check exceptions is appropriate here—update notifications should not disrupt the main install flow. The broad exception catch at line 247 is justified for this use case.
258-272: LGTM!Channel management methods are straightforward and include proper error handling.
607-616: LGTM!CLI argument parser additions for update and channel commands are well-structured with appropriate help text and argument constraints.
654-660: LGTM!Command dispatch logic correctly routes update and channel subcommands to the appropriate handler methods.
cortex/versioning.py (5)
1-19: LGTM!Module structure with clear exports and appropriate imports.
22-38: LGTM!The
CortexVersiondataclass provides a clean wrapper for version handling with proper validation infrom_string.
41-60: LGTM!The fallback chain in
get_installed_versionis well-designed: metadata lookup → package attribute → hardcoded fallback. The broad exception catch at line 57 is appropriate for the final fallback case.
63-66: LGTM!Simple and correct version comparison logic.
11-11: I apologize, but I'm unable to complete the verification due to repository access limitations. The codebase repository could not be cloned in the sandbox environment, and web searches did not return specific information about the cortexlinux/cortex project's dependency files.However, I can provide a rewritten review comment based on the original concern:
Verify
packaginglibrary is declared in project dependencies.The code imports
from packaging.version(line 11), but thepackagingdependency must be explicitly declared inpyproject.toml,setup.py,setup.cfg, orrequirements.txt. Confirm this dependency is included before merging.cortex/updater.py (12)
28-33: LGTM!Configuration constants are well-defined with sensible defaults, including XDG-compliant paths and reasonable cache TTL.
35-53: LGTM!Result dataclasses provide clear contracts for update check and perform operations with all necessary fields.
55-65: LGTM!Exception hierarchy is well-designed with specific exception types for checksum and installation failures.
67-82: LGTM!Constructor properly initializes service with environment variable fallbacks and ensures required directories exist.
84-111: LGTM!State persistence and channel management are well-implemented with atomic writes (temp file + replace) and appropriate error handling. The broad exception catch at line 90 is justified for corrupted state files.
114-172: LGTM!Update checking logic correctly implements caching with TTL, manifest fetching, and release selection. The cache behavior prevents excessive network requests while allowing force refreshes.
175-231: LGTM!The
perform_updatemethod implements a robust update workflow with proper dry-run support, rollback on failure, and cleanup in the finally block. Error handling correctly catchesUpdateErrorsubtypes and ensures cleanup.
234-260: LGTM!Download and checksum verification are correctly implemented with streaming download and chunked hashing. SHA-256 provides strong integrity guarantees.
262-283: LGTM!Installation and rollback logic correctly use
pipviasubprocess, and the rollback mechanism provides a safety net for failed upgrades.
270-283: Subprocess execution is safe; static analysis warning is a false positive.Line 274: The subprocess security warning (S603) is a false positive. The command uses
sys.executable(the current Python interpreter) and user-controlledargsfor pip operations, which is the standard safe pattern for self-updating applications.
285-296: LGTM!Upgrade recording and logging are well-implemented with proper timestamps and file handling.
299-320: LGTM!Release serialization helper correctly converts
ReleaseEntryobjects to dictionary format for JSON persistence.cortex/update_manifest.py (7)
1-16: LGTM!Clean module structure with appropriate imports for manifest parsing and version handling.
18-28: LGTM!
UpdateChannelenum withfrom_stringfactory provides type-safe channel handling with helpful error messages.
31-45: LGTM!
SystemInfodataclass correctly captures system environment details, and thecurrent()factory uses platform detection to populate fields.
48-54: LGTM!The
_detect_distrohelper gracefully handles the optionaldistropackage. The broad exception catch at line 53 is appropriate since distro detection is non-critical.
57-94: LGTM!
CompatibilityRulecorrectly validates Python version specifiers and performs multi-dimensional compatibility checks (Python version, OS, architecture, distro).
97-127: LGTM!
ReleaseEntrydataclass properly encapsulates release metadata with version, channel, download URL, checksum, and compatibility rules. Theis_compatiblecheck correctly treats empty compatibility lists as "compatible with all."
129-177: LGTM!
UpdateManifestprovides excellent manifest processing with:
from_dictfor parsing JSON manifestsiter_releasesfor filtered iterationfind_latestfor selecting the newest compatible releaseThe logic correctly filters by channel and system compatibility, then sorts by version to find the latest.
| try: | ||
| __version__ = metadata.version("cortex-linux") | ||
| except metadata.PackageNotFoundError: | ||
| __version__ = "0.1.0" | ||
|
|
||
| __all__ = ["__version__", "main", "PackageManager", "PackageManagerType"] | ||
| __version__ = "0.1.0" | ||
|
|
||
| __all__ = ["main", "PackageManager", "PackageManagerType"] |
There was a problem hiding this comment.
Critical: Dead code overrides dynamic version retrieval.
Lines 12-14 are leftover from the previous implementation and should be removed. The duplicate __version__ = "0.1.0" assignment at line 12 overwrites the dynamically retrieved version from line 7, breaking the intended behavior. The duplicate __all__ at line 14 is also unreachable.
Apply this diff to remove the dead code:
try:
__version__ = metadata.version("cortex-linux")
except metadata.PackageNotFoundError:
__version__ = "0.1.0"
__all__ = ["__version__", "main", "PackageManager", "PackageManagerType"]
-__version__ = "0.1.0"
-
-__all__ = ["main", "PackageManager", "PackageManagerType"]🧰 Tools
🪛 Ruff (0.14.6)
11-11: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
14-14: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🤖 Prompt for AI Agents
In cortex/__init__.py around lines 6 to 14, remove the dead-code duplicate
assignments that override the dynamic version and duplicate __all__: delete the
second __version__ = "0.1.0" and the second __all__ declaration so the
try/except metadata.version retrieval remains effective and only the intended
__all__ is exported.
| | Feature | Description | | ||
| |---------|-------------| | ||
| | **Natural Language Commands** | System understands intent, not syntax | | ||
| | **Hardware-Aware Optimization** | Automatically configures for your GPU/CPU | | ||
| | **Self-Healing Configuration** | Fixes broken dependencies automatically | | ||
| | **Enterprise-Grade Security** | AI actions are sandboxed and validated | | ||
| | **Installation History** | Track and rollback any installation | | ||
|
|
There was a problem hiding this comment.
Remove duplicate Feature table.
Lines 63-70 duplicate the Feature table already present at lines 55-61. Remove the duplicate.
| **Installation History** | Track and rollback any installation |
-| Feature | Description |
-|---------|-------------|
-| **Natural Language Commands** | System understands intent, not syntax |
-| **Hardware-Aware Optimization** | Automatically configures for your GPU/CPU |
-| **Self-Healing Configuration** | Fixes broken dependencies automatically |
-| **Enterprise-Grade Security** | AI actions are sandboxed and validated |
-| **Installation History** | Track and rollback any installation |
-
---
## Features📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | Feature | Description | | |
| |---------|-------------| | |
| | **Natural Language Commands** | System understands intent, not syntax | | |
| | **Hardware-Aware Optimization** | Automatically configures for your GPU/CPU | | |
| | **Self-Healing Configuration** | Fixes broken dependencies automatically | | |
| | **Enterprise-Grade Security** | AI actions are sandboxed and validated | | |
| | **Installation History** | Track and rollback any installation | |
🤖 Prompt for AI Agents
In README.md around lines 63 to 70 there is a duplicated Feature table (same
content as lines 55-61); remove the duplicate block (lines 63-70) so only the
original table at lines 55-61 remains, ensure surrounding Markdown spacing is
preserved and no extra blank lines are left after deletion.
|
@dhvll This PR has merge conflicts. Could you rebase it against main? Let me know if you need any help. Thanks! |
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
README.md (2)
63-70: Remove duplicate Feature table.Lines 63–70 duplicate the Feature table at lines 55–61. Remove the duplicate to eliminate redundant content.
| **Installation History** | Track and rollback any installation | -| Feature | Description | -|---------|-------------| -| **Natural Language Commands** | System understands intent, not syntax | -| **Hardware-Aware Optimization** | Automatically configures for your GPU/CPU | -| **Self-Healing Configuration** | Fixes broken dependencies automatically | -| **Enterprise-Grade Security** | AI actions are sandboxed and validated | -| **Installation History** | Track and rollback any installation | - ---
421-438: Restructure malformed FAQ block.The
<details>block for "What operating systems are supported?" is not properly closed before the update documentation section. The update content and "Join the Community" text should appear outside the FAQ block. The OS support answer (line 437) should immediately follow the question.<details> <summary><strong>What operating systems are supported?</strong></summary> +Currently Ubuntu 24.04 LTS with other Debian-based distributions coming soon. +</details> + ## Keeping Cortex Up to Date - Cortex automatically checks for new releases (stable by default) when you run `cortex install ...`. Disable with `CORTEX_UPDATE_CHECK=0`. - See the current channel or switch tracks: - `cortex channel show` - `cortex channel set beta` - Upgrade in-place with release notes, checksum verification, and automatic rollback on failure: - `cortex update` (use `--dry-run` to preview, `--channel beta` to override per run). - Update metadata lives in `~/.config/cortex/updater/`, including logs and last upgrade state. -### Join the Community -Currently Ubuntu 24.04 LTS. Other Debian-based distributions coming soon. -</details>
🧹 Nitpick comments (1)
README.md (1)
372-372: Consider more formal language in code example.The word "amazing" in the git commit example is colloquial. Consider using a more formal alternative like "important", "excellent", or simply "Add some AmazingFeature" to match the placeholder style used elsewhere.
-2. **Create** a feature branch (`git checkout -b feature/amazing-feature`) -3. **Commit** your changes (`git commit -m 'Add amazing feature'`) +2. **Create** a feature branch (`git checkout -b feature/important-feature`) +3. **Commit** your changes (`git commit -m 'Add important feature'`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~372-~372: Consider using a more formal and expressive alternative to ‘amazing’.
Context: ...g-feature) 3. **Commit** your changes (git commit -m 'Add amazing feature') 4. **Push** to the branch (...
(AWESOME)
🪛 markdownlint-cli2 (0.18.1)
README.md
7-7: No empty links
(MD042, no-empty-links)
246-246: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
333-333: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
498-498: Multiple headings with the same content
(MD024, no-duplicate-heading)
| ## Architecture | ||
| ``` |
There was a problem hiding this comment.
Add language specification to code block.
The code block at line 246 (Architecture diagram) is missing a language identifier. Add text for clarity.
-```
+```text
User Input🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
246-246: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In README.md around line 246, the fenced code block containing the "Architecture
diagram" is missing a language identifier; update the opening triple-backtick to
include "text" (e.g., ```text) so the block reads as a plain text code block for
clarity and proper rendering.
|
|
||
| ### Project Structure | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language specification to code block.
The code block at line 333 (Project structure) is missing a language identifier. Add text for clarity.
-```
+```text
cortex/🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
333-333: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In README.md around line 333, the fenced code block for "Project structure" has
no language tag; update the opening fence to include the language identifier
`text` (i.e., change ``` to ```text) so the block is explicitly marked as plain
text for clarity and proper syntax highlighting.
| ## Features | ||
|
|
||
| ### Core Capabilities | ||
|
|
||
| - **Natural Language Parsing** - "Install Python for machine learning" just works | ||
| - **Multi-Provider LLM Support** - Claude (Anthropic) and OpenAI GPT-4 | ||
| - **Intelligent Package Management** - Wraps apt/yum/dnf with semantic understanding | ||
| - **Hardware Detection** - Automatic GPU, CPU, RAM, storage profiling | ||
| - **Sandboxed Execution** - Firejail-based isolation for all commands | ||
| - **Installation Rollback** - Undo any installation with one command | ||
| - **Error Analysis** - AI-powered error diagnosis and fix suggestions | ||
|
|
||
| ### Supported Software (32+ Categories) |
There was a problem hiding this comment.
Remove duplicate Features section.
Lines 498–510 duplicate the Features section already present at lines 73–95. Remove this duplicate block to avoid content redundancy and violating the Markdown no-duplicate-heading rule (MD024).
---
-## Features
-
-### Core Capabilities
-
-- **Natural Language Parsing** - "Install Python for machine learning" just works
-- **Multi-Provider LLM Support** - Claude (Anthropic) and OpenAI GPT-4
-- **Intelligent Package Management** - Wraps apt/yum/dnf with semantic understanding
-- **Hardware Detection** - Automatic GPU, CPU, RAM, storage profiling
-- **Sandboxed Execution** - Firejail-based isolation for all commands
-- **Installation Rollback** - Undo any installation with one command
-- **Error Analysis** - AI-powered error diagnosis and fix suggestions
-
-### Supported Software (32+ Categories)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Features | |
| ### Core Capabilities | |
| - **Natural Language Parsing** - "Install Python for machine learning" just works | |
| - **Multi-Provider LLM Support** - Claude (Anthropic) and OpenAI GPT-4 | |
| - **Intelligent Package Management** - Wraps apt/yum/dnf with semantic understanding | |
| - **Hardware Detection** - Automatic GPU, CPU, RAM, storage profiling | |
| - **Sandboxed Execution** - Firejail-based isolation for all commands | |
| - **Installation Rollback** - Undo any installation with one command | |
| - **Error Analysis** - AI-powered error diagnosis and fix suggestions | |
| ### Supported Software (32+ Categories) | |
| --- | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
498-498: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 Prompt for AI Agents
In README.md around lines 498 to 510, remove the duplicated "Features" block
(the repeated heading and list) that mirrors the content at lines 73–95; delete
those lines so the file contains only the original Features section, ensure the
surrounding markdown flows correctly (no leftover extra blank lines or duplicate
headings), and run a quick markdown lint to confirm MD024 is resolved.
|
@mikejmorgan-ai I have rebase, can we merge now ? |
1 similar comment
|
@mikejmorgan-ai I have rebase, can we merge now ? |
5 participants
Summary
cortex/versioning.py,update_manifest.py,updater.py) that fetches manifests, enforces compatibility, downloads artifacts with SHA256 verification, and installs them via pip with automatic rollback.cortex update(--dry-run,--force,--channel) pluscortex channel show/set, and wired startup notifications intocortex install.test/test_update_service.py) covering manifest selection, channel persistence, and dry-run behavior.Checking & Installing Updates
Testing
Type of Change
Checklist
Testing
Screencast.from.18-11-25.07.28.20.PM.IST.webm
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.