Add CLI interface for cortex command - Fixes #11 #18
Conversation
|
This is excellent - the CLI is the user's first interaction with Cortex Linux, so this is critical. Before merge, need to validate:
Once I can verify this works end-to-end, this merges immediately. Re: Issue #11 - Was there an Issue #11 created, or should this be fixing a different issue number? Also - you've now delivered:
If this CLI PR is substantial, I'm adding another $150-200 bounty for it. Let me test it first. Great work building the user experience layer. |
|
Yes ofcourse you can test it, I have considered all this and created a dedicated test file as well for it. For your simplicity and testing just run |
|
@dhvll Can you pull this branch and run python test_cli.py to verify integration? Want technical sign-off before merge." |
|
Yes sure will do it @mikejmorgan-ai |
|
@Sahilbhatane please attach a video of the implementation. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis change introduces a complete CLI-based software installation system for the Cortex project. It adds packaging infrastructure (setup.py, MANIFEST.in, .gitignore), a new CortexCLI command-line interface with interactive installation workflows, an InstallationCoordinator framework for managing multi-step installations with progress tracking and rollback support, comprehensive test coverage, and adjusts the LLM module's default model configuration. Changes
Sequence DiagramsequenceDiagram
participant User
participant CortexCLI
participant CommandInterpreter
participant InstallationCoordinator
participant Shell
User->>CortexCLI: cortex install docker --execute
CortexCLI->>CortexCLI: Get API key (env vars)
CortexCLI->>CortexCLI: Detect provider (openai/claude)
rect rgb(220, 240, 250)
Note over CortexCLI: Planning Phase
CortexCLI->>CommandInterpreter: Generate install commands
CommandInterpreter-->>CortexCLI: List of shell commands
end
alt Dry-run Mode
CortexCLI->>User: Display commands (no execution)
CortexCLI-->>User: Exit (code 0)
else Execute Mode
rect rgb(240, 230, 250)
Note over CortexCLI,InstallationCoordinator: Execution Phase
CortexCLI->>InstallationCoordinator: execute()
loop For each command
InstallationCoordinator->>Shell: Run command
Shell-->>InstallationCoordinator: Output + return code
InstallationCoordinator->>CortexCLI: progress_callback (step status)
CortexCLI->>User: Print status (SUCCESS/FAILED)
end
end
InstallationCoordinator-->>CortexCLI: InstallationResult
alt Success
CortexCLI->>User: ✅ Installation complete
else Failure
CortexCLI->>User: ❌ Installation failed
opt Rollback enabled
InstallationCoordinator->>Shell: Execute rollback commands
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a command-line interface for Cortex, enabling users to install software using natural language commands. The CLI integrates with the existing LLM layer to parse user requests and generate validated system commands, with options for dry-run mode and actual execution.
Key Changes
- CLI entry point with
cortex install <software>command supporting dry-run and execute modes - Installation coordinator for multi-step command execution with progress tracking, rollback support, and logging
- Comprehensive test suites for both CLI and coordinator components with 100+ unit tests
- Package configuration with setup.py for pip installation and console script registration
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 27 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Configures pip-installable package with console script entry point and dependencies from LLM/requirements.txt |
| cortex/cli.py | Main CLI implementation with natural language parsing, progress indicators, and command execution modes |
| cortex/coordinator.py | Multi-step installation coordinator with progress tracking, rollback, verification, and logging capabilities |
| cortex/init.py | Package initialization with version and main entry point export |
| cortex/test_cli.py | Unit tests for CLI functionality covering all execution modes and error scenarios |
| cortex/test_coordinator.py | Unit tests for coordinator covering command execution, rollback, verification, and edge cases |
| MANIFEST.in | Package manifest for distribution including README, LICENSE, and source files |
| LLM/test_interpreter.py | Updated test imports to use absolute module paths for better compatibility |
| .gitignore | Standard Python gitignore patterns for build artifacts and virtual environments |
Comments suppressed due to low confidence (6)
cortex/coordinator.py:5
- Import of 'field' is not used.
from dataclasses import dataclass, field
cortex/cli.py:5
- Import of 'List' is not used.
from typing import List, Optional
cortex/cli.py:6
- Import of 'subprocess' is not used.
import subprocess
cortex/test_coordinator.py:2
- Import of 'call' is not used.
from unittest.mock import Mock, patch, call
cortex/test_cli.py:2
- Import of 'call' is not used.
Import of 'MagicMock' is not used.
from unittest.mock import Mock, patch, MagicMock, call
cortex/coordinator.py:82
- 'except' clause does nothing but pass and there is no explanatory comment.
except Exception:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (9)
cortex/test_cli.py (5)
2-2: Remove unused imports.The
MagicMockandcallimports are not used in this test file.Apply this diff to remove unused imports:
-from unittest.mock import Mock, patch, MagicMock, call +from unittest.mock import Mock, patch
199-202: Mocktime.sleepto speed up test execution.The
_animate_spinnermethod includestime.sleep(0.1), making this test unnecessarily slow. Mocking the sleep call eliminates the delay while still verifying the spinner logic.Apply this diff to mock the sleep:
- def test_spinner_animation(self): + @patch('time.sleep', return_value=None) + def test_spinner_animation(self, mock_sleep): initial_idx = self.cli.spinner_idx self.cli._animate_spinner("Testing") self.assertNotEqual(self.cli.spinner_idx, initial_idx)
42-46: Fix ineffective test assertion.The assertion
self.assertTrue(mock_stdout.write.called or print)always evaluates to True becauseApply this diff to fix the assertion:
@patch('sys.stdout') def test_print_status(self, mock_stdout): self.cli._print_status("🧠", "Test message") - self.assertTrue(mock_stdout.write.called or print) + # _print_status uses print(), which writes to stdout + # Verify print was called (print calls stdout.write) + self.assertTrue(mock_stdout.write.called)
47-51: Fix ineffective test assertion.The assertion
self.assertTrue(True)always passes and doesn't verify anything. The test should check that the error message was written to stderr.Apply this diff to verify the mock was called:
@patch('sys.stderr') def test_print_error(self, mock_stderr): self.cli._print_error("Test error") - self.assertTrue(True) + # Verify error was written to stderr + mock_stderr.write.assert_called()
52-56: Fix ineffective test assertion.The assertion
self.assertTrue(True)always passes and doesn't verify anything. The test should check that the success message was written to stdout.Apply this diff to verify the mock was called:
@patch('sys.stdout') def test_print_success(self, mock_stdout): self.cli._print_success("Test success") - self.assertTrue(True) + # Verify success message was written to stdout + mock_stdout.write.assert_called()cortex/coordinator.py (4)
5-5: Remove unused import.The
fieldimport from dataclasses is not used in this file.Apply this diff:
-from dataclasses import dataclass, field +from dataclasses import dataclass
85-126: Consider security implications ofshell=True.Using
shell=True(line 94) with user-controlled input enables shell injection vulnerabilities if the LLM is manipulated or validation is bypassed. While commands are validated by the LLM layer, usingshell=Falsewith parsed command lists (e.g., viashlex.split()) would provide defense-in-depth.Consider this approach for better security:
import shlex # In _execute_command: try: result = subprocess.run( shlex.split(step.command), # Parse command safely shell=False, capture_output=True, text=True, timeout=self.timeout )Note: This requires commands to be simple (no pipes, redirects, shell expansions). For complex shell commands, additional parsing or a different approach would be needed.
155-159: Progress callback timing consideration.The progress callback is invoked at line 157 before
_execute_commandruns, so the step status is stillPENDING. After execution, the status updates toSUCCESSorFAILED, but the callback has already been called. As noted in past reviews, this means the callback receives pre-execution state. Consider calling the callback after execution for accurate status reporting, or document this timing behavior clearly.
248-284: Example function demonstrates coordinator usage well, but uses deprecated command.The
install_docker()function provides a clear example of coordinator usage with descriptions, timeout, and verification. However, line 252 uses the deprecatedapt-key addcommand. Modern Ubuntu versions (22.04+) prefer storing keys in/etc/apt/keyrings/.Consider updating to the modern approach:
- "curl -fsSL https://download.docker.com/linux/ubuntu/gpg | apt-key add -", + "curl -fsSL https://download.docker.com/linux/ubuntu/gpg | gpg --dearmor -o /etc/apt/keyrings/docker.gpg",And update the repository addition command accordingly:
- 'add-apt-repository "deb [arch=amd64] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable"', + 'echo "deb [arch=amd64 signed-by=/etc/apt/keyrings/docker.gpg] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable" > /etc/apt/sources.list.d/docker.list',
🧹 Nitpick comments (1)
cortex/cli.py (1)
8-11: Drop runtime sys.path mutationHard-coding
sys.path.insert(...)in production CLI code is unnecessary once the package is installed and risks precedence bugs. The package layout and entry point already ensure bothcortexandLLMare importable. Please remove the path tweaking and rely on proper packaging.-sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) - from LLM.interpreter import CommandInterpreter
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.gitignore(1 hunks)LLM/interpreter.py(1 hunks)LLM/test_interpreter.py(10 hunks)MANIFEST.in(1 hunks)cortex/__init__.py(1 hunks)cortex/cli.py(1 hunks)cortex/coordinator.py(1 hunks)cortex/test_cli.py(1 hunks)cortex/test_coordinator.py(1 hunks)setup.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
cortex/coordinator.py (1)
cortex/cli.py (1)
progress_callback(86-93)
cortex/test_cli.py (3)
cortex/cli.py (9)
CortexCLI(14-133)main(136-172)_get_api_key(19-24)_get_provider(26-31)_print_status(33-34)_print_error(36-37)_print_success(39-40)install(52-133)_animate_spinner(42-46)LLM/interpreter.py (1)
parse(131-145)cortex/coordinator.py (1)
execute(149-195)
cortex/__init__.py (1)
cortex/cli.py (1)
main(136-172)
cortex/test_coordinator.py (1)
cortex/coordinator.py (11)
InstallationCoordinator(44-245)InstallationStep(19-32)InstallationResult(36-41)StepStatus(10-15)install_docker(248-284)duration(29-32)execute(149-195)add_rollback_command(146-147)verify_installation(197-218)get_summary(220-241)export_log(243-245)
LLM/test_interpreter.py (1)
LLM/interpreter.py (3)
CommandInterpreter(12-158)APIProvider(7-9)parse(131-145)
cortex/cli.py (2)
LLM/interpreter.py (2)
CommandInterpreter(12-158)parse(131-145)cortex/coordinator.py (3)
InstallationCoordinator(44-245)StepStatus(10-15)execute(149-195)
🪛 Ruff (0.14.4)
cortex/coordinator.py
62-62: Avoid specifying long messages outside the exception class
(TRY003)
82-83: try-except-pass detected, consider logging the exception
(S110)
82-82: Do not catch blind exception: Exception
(BLE001)
92-92: subprocess call with shell=True identified, security issue
(S602)
121-121: Do not catch blind exception: Exception
(BLE001)
125-125: Use explicit conversion flag
Replace with conversion flag
(RUF010)
137-137: subprocess call with shell=True identified, security issue
(S602)
143-143: Do not catch blind exception: Exception
(BLE001)
144-144: Use explicit conversion flag
Replace with conversion flag
(RUF010)
204-204: subprocess call with shell=True identified, security issue
(S602)
214-214: Do not catch blind exception: Exception
(BLE001)
216-216: Use explicit conversion flag
Replace with conversion flag
(RUF010)
cortex/test_cli.py
28-28: Unused method argument: mock_stderr
(ARG002)
48-48: Unused method argument: mock_stderr
(ARG002)
53-53: Unused method argument: mock_stdout
(ARG002)
LLM/test_interpreter.py
82-82: Unused method argument: mock_openai
(ARG002)
cortex/cli.py
123-123: Consider moving this statement to an else block
(TRY300)
129-129: Use explicit conversion flag
Replace with conversion flag
(RUF010)
131-131: Do not catch blind exception: Exception
(BLE001)
132-132: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (8)
cortex/test_cli.py (1)
57-202: Excellent test coverage.The test suite comprehensively covers:
- API key detection and provider selection
- All execution modes (dry-run, execute, default)
- Success and failure paths
- Exception handling (ValueError, RuntimeError, generic Exception)
- Edge cases (no commands generated, missing API key)
- CLI argument parsing and main entry point
This provides strong confidence in the CLI implementation.
cortex/coordinator.py (7)
10-42: Well-designed data structures.The
StepStatusenum and dataclasses (InstallationStep,InstallationResult) provide clear state tracking with timing, output capture, and return codes. Theduration()method is a nice ergonomic touch.
44-72: Solid initialization with validation.The constructor properly validates that descriptions match commands and initializes all necessary state. The flexible configuration options (timeout, stop_on_error, rollback, callbacks) provide good control.
74-83: Logging exception handling is acceptable.While the broad exception handler at lines 82-83 silently suppresses logging errors, this is appropriate for auxiliary logging functionality. Failing to write logs shouldn't crash the installation process.
128-147: Rollback mechanism is well-implemented.The rollback logic correctly executes commands in reverse order and handles failures gracefully. The same
shell=Truesecurity consideration from the previous comment applies here as well.
149-195: Execute method orchestrates installation flow correctly.The method properly:
- Tracks timing and failed steps
- Invokes progress callbacks
- Handles stop-on-error with step skipping
- Triggers rollback when enabled
- Constructs detailed results with all step information
The overall control flow is solid.
197-218: Verification method provides useful validation.The verification mechanism runs commands post-installation and reports pass/fail status. This is helpful for confirming successful installation. The same
shell=Truesecurity consideration applies here.
220-245: Summary and export utilities enhance observability.The
get_summary()method provides clear aggregated statistics and per-step details, whileexport_log()enables persistent audit trails. These are valuable for debugging and monitoring installation processes.
|
Open issue #8 in separate PR. And make a habit of uploading video demonstration of your code is working. Just test cases are not enough. It becomes easier for reviewer to review the PR. |
|
Great progress! The core implementation is solid. To complete this PR, please:
This is the most critical feature for Cortex Linux. Your work is laying the foundation for everything else. Keep going! 💪 Bounty remains $100 on merge. Additional $25 bonus if you include working demo. |
|
@dhvll and @Sahilbhatane - PR #18 is substantial work on our CLI interface. Review assignments:
Both sign-offs required before merge. This is a critical foundation piece. Bounty: $100 each on approval and merge. |
|
|
Saw PR #18 (CLI interface) has a lot of discussion happening. This is your call as Lead
I trust your technical judgment. You're the decision-maker on all code reviews now. Just keep me updated on:
Otherwise, you run the technical side. I'm focused on fundraising and business development.
|
|
I'm personally working on that issue because it is critical to our project. |
|
Warning Docstrings generation - IN PROGRESS Generating docstrings for this pull request |
3 participants
Simple CLI Interface
Fixes #11
Summary
Implemented a command-line interface for Cortex that allows users to install software using natural language commands. The CLI integrates with the existing LLM layer to parse requests and generate validated system commands.
Implementation
Core Features
cortex install <software>command--execute: Runs the generated commands--dry-run: Displays commands for reviewFiles Created
cortex/cli.py- Main CLI implementationCortexCLIclass with install command logiccortex/__init__.py- Package initializationcortex/test_cli.py- Test suitesetup.py- Package configurationcortexMANIFEST.in- Package manifest.gitignore- Git ignore rulesUsage Examples