Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 116 additions & 4 deletions cortex/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,9 @@ def notify(self, args):

elif args.notify_action == "enable":
mgr.config["enabled"] = True
# Addressing CodeRabbit feedback: Ideally should use a public method instead of private _save_config,
# but keeping as is for a simple fix (or adding a save method to NotificationManager would be best).
# Addressing CodeRabbit feedback: Ideally should use a public method
# instead of private _save_config, but keeping as is for a simple fix
# (or adding a save method to NotificationManager would be best).
mgr._save_config()
self._print_success("Notifications enabled")
return 0
Expand Down Expand Up @@ -556,6 +557,9 @@ def install(
execute: bool = False,
dry_run: bool = False,
parallel: bool = False,
from_source: bool = False,
source_url: str | None = None,
version: str | None = None,
):
# Validate input first
is_valid, error = validate_install_request(software)
Expand Down Expand Up @@ -590,6 +594,12 @@ def install(
start_time = datetime.now()

try:
# Handle --from-source flag
if from_source:
return self._install_from_source(
software, execute, dry_run, source_url, version
)

self._print_status("🧠", "Understanding request...")

interpreter = CommandInterpreter(api_key=api_key, provider=provider)
Expand Down Expand Up @@ -878,7 +888,8 @@ def history(self, limit: int = 20, status: str | None = None, show_id: str | Non
packages += f" +{len(r.packages) - 2}"

print(
f"{r.id:<18} {date:<20} {r.operation_type.value:<12} {packages:<30} {r.status.value:<15}"
f"{r.id:<18} {date:<20} {r.operation_type.value:<12} "
f"{packages:<30} {r.status.value:<15}"
Comment on lines +891 to +892
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Apply Black formatting to resolve pipeline failure.

The CI pipeline reports a Black formatting check failure. Run black . to auto-format the code before merging.

#!/bin/bash
# Verify Black formatting issues
black --check cortex/cli.py --diff
🤖 Prompt for AI Agents
In cortex/cli.py around lines 891-892, the CI failed Black formatting on the
f-string lines; run Black to reformat the file and commit the changes (e.g., run
`black .` or `black cortex/cli.py`), then re-run the formatting check to ensure
the f-string line breaks and spacing conform to Black before pushing the updated
file.

)

return 0
Expand Down Expand Up @@ -1192,7 +1203,8 @@ def _env_template(self, env_mgr: EnvironmentManager, args: argparse.Namespace) -
return self._env_template_apply(env_mgr, args)
else:
self._print_error(
"Please specify: template list, template show <name>, or template apply <name> <app>"
"Please specify: template list, template show <name>, "
"or template apply <name> <app>"
)
return 1

Expand Down Expand Up @@ -1534,6 +1546,86 @@ def progress_callback(current: int, total: int, step: InstallationStep) -> None:
console.print(f"Error: {result.error_message}", style="red")
return 1

def _install_from_source(
self,
package_name: str,
execute: bool,
dry_run: bool,
source_url: str | None,
version: str | None,
) -> int:
"""Handle installation from source."""
from cortex.source_builder import SourceBuilder

builder = SourceBuilder()

# Parse version from package name if specified (e.g., python@3.12)
if "@" in package_name and not version:
parts = package_name.split("@")
package_name = parts[0]
version = parts[1] if len(parts) > 1 else None

cx_print(f"Building {package_name} from source...", "info")
if version:
cx_print(f"Version: {version}", "info")

result = builder.build_from_source(
package_name=package_name,
version=version,
source_url=source_url,
use_cache=True,
)

if not result.success:
self._print_error(f"Build failed: {result.error_message}")
return 1

if result.cached:
cx_print(f"Using cached build for {package_name}", "info")

if dry_run:
cx_print("\nBuild commands (dry run):", "info")
for cmd in result.install_commands:
console.print(f" • {cmd}")
return 0

if not execute:
cx_print("\nBuild completed. Install commands:", "info")
for cmd in result.install_commands:
console.print(f" • {cmd}")
cx_print("Run with --execute to install", "info")
return 0

# Execute install commands
from cortex.coordinator import InstallationCoordinator, InstallationStep, StepStatus

def progress_callback(current: int, total: int, step: InstallationStep) -> None:
status_emoji = "⏳"
if step.status == StepStatus.SUCCESS:
status_emoji = "✅"
elif step.status == StepStatus.FAILED:
status_emoji = "❌"
console.print(f"[{current}/{total}] {status_emoji} {step.description}")

coordinator = InstallationCoordinator(
commands=result.install_commands,
descriptions=[f"Install {package_name}" for _ in result.install_commands],
timeout=600,
stop_on_error=True,
progress_callback=progress_callback,
)

install_result = coordinator.execute()

if install_result.success:
self._print_success(f"{package_name} built and installed successfully!")
return 0
else:
self._print_error("Installation failed")
if install_result.error_message:
console.print(f"Error: {install_result.error_message}", style="red")
return 1
Comment on lines +1549 to +1627
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Implement audit logging for source-based installations.

The method bypasses the installation history tracking that is present in the regular install path (lines 579-616, 682-686, 743-746). Source-based installations should also be logged to ~/.cortex/history.db for consistency and traceability.

🔎 Add installation history tracking
 def _install_from_source(
     self,
     package_name: str,
     execute: bool,
     dry_run: bool,
     source_url: str | None,
     version: str | None,
 ) -> int:
     """Handle installation from source."""
     from cortex.source_builder import SourceBuilder
+    from datetime import datetime

     builder = SourceBuilder()
+    history = InstallationHistory()
+    install_id = None
+    start_time = datetime.now()

     # Parse version from package name if specified (e.g., python@3.12)
     if "@" in package_name and not version:
         parts = package_name.split("@")
         package_name = parts[0]
         version = parts[1] if len(parts) > 1 else None

     cx_print(f"Building {package_name} from source...", "info")
     if version:
         cx_print(f"Version: {version}", "info")

     result = builder.build_from_source(
         package_name=package_name,
         version=version,
         source_url=source_url,
         use_cache=True,
     )

     if not result.success:
         self._print_error(f"Build failed: {result.error_message}")
+        if install_id:
+            history.update_installation(install_id, InstallationStatus.FAILED, result.error_message)
         return 1

     if result.cached:
         cx_print(f"Using cached build for {package_name}", "info")

     if dry_run:
         cx_print("\nBuild commands (dry run):", "info")
         for cmd in result.install_commands:
             console.print(f"  • {cmd}")
+        if execute or dry_run:
+            install_id = history.record_installation(
+                InstallationType.INSTALL, [package_name], result.install_commands, start_time
+            )
+            history.update_installation(install_id, InstallationStatus.SUCCESS)
         return 0

     if not execute:
         cx_print("\nBuild completed. Install commands:", "info")
         for cmd in result.install_commands:
             console.print(f"  • {cmd}")
         cx_print("Run with --execute to install", "info")
         return 0

+    # Record installation start
+    install_id = history.record_installation(
+        InstallationType.INSTALL, [package_name], result.install_commands, start_time
+    )
+
     # Execute install commands
     from cortex.coordinator import InstallationCoordinator, InstallationStep, StepStatus

     def progress_callback(current: int, total: int, step: InstallationStep) -> None:
         status_emoji = "⏳"
         if step.status == StepStatus.SUCCESS:
             status_emoji = "✅"
         elif step.status == StepStatus.FAILED:
             status_emoji = "❌"
         console.print(f"[{current}/{total}] {status_emoji} {step.description}")

     coordinator = InstallationCoordinator(
         commands=result.install_commands,
         descriptions=[f"Install {package_name}" for _ in result.install_commands],
         timeout=600,
         stop_on_error=True,
         progress_callback=progress_callback,
     )

     install_result = coordinator.execute()

     if install_result.success:
         self._print_success(f"{package_name} built and installed successfully!")
+        if install_id:
+            history.update_installation(install_id, InstallationStatus.SUCCESS)
+            print(f"\n📝 Installation recorded (ID: {install_id})")
+            print(f"   To rollback: cortex rollback {install_id}")
         return 0
     else:
         self._print_error("Installation failed")
+        if install_id:
+            error_msg = install_result.error_message or "Installation failed"
+            history.update_installation(install_id, InstallationStatus.FAILED, error_msg)
+            print(f"\n📝 Installation recorded (ID: {install_id})")
+            print(f"   View details: cortex history {install_id}")
         if install_result.error_message:
             console.print(f"Error: {install_result.error_message}", style="red")
         return 1

Based on learnings: Implement audit logging to ~/.cortex/history.db for all package operations.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In cortex/cli.py around lines 1536-1614, the source-install path skips writing
to the ~/.cortex/history.db audit log; update the flow to record installation
history just like the regular install paths: after a build succeeds and before
returning success, call the same history persistence routine used elsewhere (or
import and use the HistoryDB/HistoryStore helper) to insert an entry with
package_name, version, source_url, method="source", timestamp, result="success";
likewise, when installation fails (install_result.success is False) record an
entry with result="failed" and include the error_message; ensure entries are
written for dry-run/cached cases as appropriate using the same schema and
storage location (~/.cortex/history.db) and place the calls immediately before
the early return statements so audit logging always runs.


# --------------------------


Expand Down Expand Up @@ -1654,6 +1746,23 @@ def main():
action="store_true",
help="Enable parallel execution for multi-step installs",
)
install_parser.add_argument(
"--from-source",
action="store_true",
help=(
"Build and install from source code when binaries unavailable"
),
)
install_parser.add_argument(
"--source-url",
type=str,
help="URL to source code (for --from-source)",
)
install_parser.add_argument(
"--version",
type=str,
help="Version to build (for --from-source)",
)

# Import command - import dependencies from package manager files
import_parser = subparsers.add_parser(
Expand Down Expand Up @@ -1895,6 +2004,9 @@ def main():
execute=args.execute,
dry_run=args.dry_run,
parallel=args.parallel,
from_source=getattr(args, "from_source", False),
source_url=getattr(args, "source_url", None),
version=getattr(args, "version", None),
Comment on lines +2007 to +2009
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Update parameter name when --version is renamed.

Once the install subparser's --version argument is renamed to --pkg-version (see comment on lines 1749-1763), update line 2007 accordingly:

                 from_source=getattr(args, "from_source", False),
                 source_url=getattr(args, "source_url", None),
-                version=getattr(args, "version", None),
+                version=getattr(args, "pkg_version", None),
             )
📝 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.

Suggested change
from_source=getattr(args, "from_source", False),
source_url=getattr(args, "source_url", None),
version=getattr(args, "version", None),
from_source=getattr(args, "from_source", False),
source_url=getattr(args, "source_url", None),
version=getattr(args, "pkg_version", None),
🤖 Prompt for AI Agents
In cortex/cli.py around lines 2005 to 2007, the call is still using
getattr(args, "version", None) but the install subparser renamed the CLI flag to
--pkg-version; update this line to use getattr(args, "pkg_version", None) (and
keep from_source and source_url as-is) so the code reads the new parameter name
from args.

)
elif args.command == "import":
return cli.import_deps(args)
Expand Down
Loading
Loading