-
-
Notifications
You must be signed in to change notification settings - Fork 50
feat: add interactive systemd service generator (Bounty #448) #461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1521,6 +1521,18 @@ def progress_callback(current: int, total: int, step: InstallationStep) -> None: | |
| console.print(f"Error: {result.error_message}", style="red") | ||
| return 1 | ||
|
|
||
| def systemd(self) -> int: | ||
| """Interactive systemd service generator.""" | ||
| try: | ||
| from cortex.systemd_helper import SystemdHelper | ||
|
|
||
| helper = SystemdHelper() | ||
| helper.run() | ||
| return 0 | ||
| except Exception as e: | ||
| self._print_error(f"Systemd Helper failed: {e}") | ||
| return 1 | ||
|
Comment on lines
+1524
to
+1534
|
||
|
|
||
| # -------------------------- | ||
|
|
||
|
|
||
|
|
@@ -1553,6 +1565,7 @@ def show_rich_help(): | |
| table.add_row("cache stats", "Show LLM cache statistics") | ||
| table.add_row("stack <name>", "Install the stack") | ||
| table.add_row("sandbox <cmd>", "Test packages in Docker sandbox") | ||
| table.add_row("systemd", "Generate systemd service files") | ||
| table.add_row("doctor", "System health check") | ||
|
|
||
| console.print(table) | ||
|
|
@@ -1857,6 +1870,11 @@ def main(): | |
| env_template_apply_parser.add_argument( | ||
| "--encrypt-keys", help="Comma-separated list of keys to encrypt" | ||
| ) | ||
|
|
||
| # Systemd helper | ||
| subparsers.add_parser( | ||
| "systemd", aliases=["service-gen"], help="Generate systemd service files" | ||
| ) | ||
| # -------------------------- | ||
|
|
||
| args = parser.parse_args() | ||
|
|
@@ -1903,6 +1921,8 @@ def main(): | |
| return 1 | ||
| elif args.command == "env": | ||
| return cli.env(args) | ||
| elif args.command in ["systemd", "service-gen"]: | ||
| return cli.systemd() | ||
| else: | ||
| parser.print_help() | ||
| return 1 | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,110 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||
| import getpass | ||||||||||||||||||||||||||||||||||||||||||||||
| from rich.prompt import Prompt, Confirm | ||||||||||||||||||||||||||||||||||||||||||||||
| from cortex.branding import console, cx_header, cx_print | ||||||||||||||||||||||||||||||||||||||||||||||
|
Check failure on line 4 in cortex/systemd_helper.py
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class SystemdHelper: | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| Interactive helper to generate systemd service files. | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| self.service_name = "" | ||||||||||||||||||||||||||||||||||||||||||||||
| self.description = "" | ||||||||||||||||||||||||||||||||||||||||||||||
| self.exec_start = "" | ||||||||||||||||||||||||||||||||||||||||||||||
| self.working_dir = "" | ||||||||||||||||||||||||||||||||||||||||||||||
| self.user = "" | ||||||||||||||||||||||||||||||||||||||||||||||
| self.restart_policy = "always" | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add type hints and docstring to The 🔎 Proposed fix- def __init__(self):
+ def __init__(self) -> None:
+ """Initialize the SystemdHelper with default values for service configuration."""
self.service_name = ""
self.description = ""
self.exec_start = ""
self.working_dir = ""
self.user = ""
self.restart_policy = "always"Based on coding guidelines, type hints and docstrings are required for all public APIs. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def run(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| """Interactive wizard.""" | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
| """Interactive wizard.""" | |
| """ | |
| Run the interactive CLI wizard to generate a systemd service file. | |
| This method interactively prompts the user for the service name, description, | |
| command to run (ExecStart), working directory, user, and restart policy. | |
| It then generates the corresponding systemd unit file content, displays it | |
| for review, and optionally writes the file to the current working directory. | |
| On successful save, it prints suggested next steps for installing and | |
| managing the service with systemd. | |
| The method does not accept any arguments other than ``self`` and always | |
| returns ``None``. | |
| File-writing errors that occur while saving the generated service file are | |
| caught and reported to the user by :meth:`save_file` and are not re-raised | |
| by this method. | |
| Raises: | |
| OSError: If the current working directory cannot be determined when | |
| calling :func:`os.getcwd`. | |
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add return type hint and improve docstring.
The run method lacks a return type hint and has a minimal docstring that doesn't describe the interactive wizard flow or its purpose in detail.
🔎 Proposed fix
- def run(self):
- """Interactive wizard."""
+ def run(self) -> None:
+ """
+ Run the interactive wizard to collect systemd service configuration.
+
+ Prompts the user for service name, description, command, working directory,
+ user, and restart policy. Generates the service file content and optionally
+ saves it to disk with follow-up instructions.
+ """Based on coding guidelines, type hints are required in Python code and docstrings are required for all public APIs.
🤖 Prompt for AI Agents
In cortex/systemd_helper.py around lines 19 to 20, the public method run() is
missing a return type hint and has an underspecified docstring; update the
signature to include an explicit return type (e.g., -> None or an appropriate
type if it returns a value) and replace the one-line docstring with a short
multi-sentence docstring that states the method is an interactive wizard,
describes its purpose, outlines the high-level flow (what prompts it shows and
what it returns or persists), and documents side effects and return value;
ensure the docstring follows project docstring conventions (reST/Google style)
and include type information for the return and any important parameters.
Check failure on line 22 in cortex/systemd_helper.py
GitHub Actions / lint
Ruff (W293)
cortex/systemd_helper.py:22:1: W293 Blank line contains whitespace
Check failure on line 27 in cortex/systemd_helper.py
GitHub Actions / lint
Ruff (W291)
cortex/systemd_helper.py:27:67: W291 Trailing whitespace
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example text shows "myserver" but this is inconsistent with the default value "myservice". Consider changing the example to match the default or vice versa for consistency.
| "[bold cyan]Service Name[/bold cyan] (e.g. myserver)", | |
| "[bold cyan]Service Name[/bold cyan] (e.g. myservice)", |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The service name extraction logic has a potential bug. When a user enters a service name ending with ".service", the code strips 8 characters (line 34), but it should strip 8 characters to get the name without the extension. However, if the user enters just ".service" or a very short name like "a.service", stripping 8 characters would leave an empty or negative-indexed string. Consider using filename.removesuffix(".service") (Python 3.9+) or filename[:-8] if len(filename) > 8 else "" with validation to ensure the service name is not empty.
| self.service_name = filename[:-8] | |
| stripped_name = filename.removesuffix(".service") | |
| # Avoid ending up with an empty service name (e.g. input ".service") | |
| if stripped_name: | |
| self.service_name = stripped_name | |
| else: | |
| self.service_name = filename |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle edge case when user enters exactly ".service" as service name.
If a user enters exactly ".service" as the service name, line 34 will set self.service_name to an empty string (".service"[:-8] = ""), which would result in an invalid service name and could cause issues later.
🔎 Proposed fix to validate service name
self.service_name = Prompt.ask(
"[bold cyan]Service Name[/bold cyan] (e.g. myserver)",
default="myservice"
)
+
+ # Strip .service suffix if present
if not self.service_name.endswith(".service"):
filename = f"{self.service_name}.service"
else:
filename = self.service_name
self.service_name = filename[:-8]
+
+ # Validate service name is not empty
+ if not self.service_name or not self.service_name.strip():
+ cx_print("Invalid service name. Using default 'myservice'", "warning")
+ self.service_name = "myservice"
+ filename = "myservice.service"🤖 Prompt for AI Agents
In cortex/systemd_helper.py around lines 26 to 34, the current logic turns an
input of ".service" into an empty service name; after reading the Prompt input,
strip whitespace, then if the input equals ".service" or the computed base name
(input with a trailing ".service" removed) is empty, treat it as invalid —
either re-prompt the user or fall back to a sane default (e.g., "myservice");
otherwise, if input endswith ".service" strip the suffix and use the remaining
non-empty name, and if it does not endwith ".service" use the stripped input,
ensuring you never assign an empty string to self.service_name.
Check failure on line 38 in cortex/systemd_helper.py
GitHub Actions / lint
Ruff (W291)
cortex/systemd_helper.py:38:50: W291 Trailing whitespace
Check failure on line 51 in cortex/systemd_helper.py
GitHub Actions / lint
Ruff (W291)
cortex/systemd_helper.py:51:56: W291 Trailing whitespace
Check failure on line 58 in cortex/systemd_helper.py
GitHub Actions / lint
Ruff (W291)
cortex/systemd_helper.py:58:50: W291 Trailing whitespace
Check failure on line 64 in cortex/systemd_helper.py
GitHub Actions / lint
Ruff (W291)
cortex/systemd_helper.py:64:53: W291 Trailing whitespace
Check failure on line 65 in cortex/systemd_helper.py
GitHub Actions / lint
Ruff (W291)
cortex/systemd_helper.py:65:52: W291 Trailing whitespace
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The restart policy choice "no" should be "no" in the generated systemd file, but systemd actually expects "Restart=no" to be omitted or use other values. The valid values for the Restart directive are: "no", "on-success", "on-failure", "on-abnormal", "on-watchdog", "on-abort", or "always". While "no" is technically valid, consider adding "on-success" or "on-abnormal" as additional commonly used options to provide users with more flexibility.
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user inputs (service_name, exec_start, working_dir, user) are not validated before being written to the service file. While systemd will validate the file when it's loaded, consider adding basic validation to catch common errors early. For example: validate that the service_name doesn't contain invalid characters (spaces, special chars), that exec_start contains an executable path, that working_dir is an absolute path, and that the user exists on the system.
Check failure on line 71 in cortex/systemd_helper.py
GitHub Actions / lint
Ruff (W293)
cortex/systemd_helper.py:71:1: W293 Blank line contains whitespace
Check failure on line 79 in cortex/systemd_helper.py
GitHub Actions / lint
Ruff (W293)
cortex/systemd_helper.py:79:1: W293 Blank line contains whitespace
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the user declines to save the file (answers "no" to the confirmation prompt), the wizard exits silently without any feedback. Consider adding an else clause to provide feedback to the user, such as informing them that the generated content was not saved or providing instructions on how to regenerate it if needed.
| console.print(f"4. Start service: [cyan]sudo systemctl start {self.service_name}[/cyan]") | |
| console.print(f"4. Start service: [cyan]sudo systemctl start {self.service_name}[/cyan]") | |
| else: | |
| cx_print("Generated service file was not saved. Rerun this wizard if you need to regenerate it.", "info") |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file saving operation lacks proper error handling and user feedback. When an exception occurs during file writing, the error is printed but the wizard continues to display "Next Steps" instructions as if the file was saved successfully. Consider returning early or raising the exception from save_file so the caller can handle the failure case appropriately and avoid showing misleading success instructions.
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new systemd helper functionality lacks test coverage. Similar CLI commands in this repository have corresponding test files (e.g., test_cli.py, test_doctor.py, test_env_manager.py). Consider adding tests for the SystemdHelper class covering the interactive wizard flow, service file generation, and file saving operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Improve save_file method with type hints, encoding, and return value.
The method has several issues:
- Missing type hints (violates coding guidelines)
- No explicit encoding specified for file writing (should be UTF-8)
- Overly broad exception handling catches all exceptions
- No return value to indicate success/failure to caller
🔎 Proposed improvements
- def save_file(self, filename: str, content: str):
- """Saves content to file."""
+ def save_file(self, filename: str, content: str) -> bool:
+ """
+ Save content to file.
+
+ Args:
+ filename: Path where the file should be saved
+ content: Content to write to the file
+
+ Returns:
+ True if file was saved successfully, False otherwise
+ """
try:
- with open(filename, "w") as f:
+ with open(filename, "w", encoding="utf-8") as f:
f.write(content)
- except Exception as e:
+ return True
+ except (OSError, IOError) as e:
cx_print(f"Error saving file: {e}", "error")
+ return FalseBased on coding guidelines, type hints are required in Python code.
🤖 Prompt for AI Agents
In cortex/systemd_helper.py around lines 104 to 110, the save_file method lacks
type hints, does not specify file encoding, catches all exceptions too broadly,
and returns nothing; change the signature to include type hints for parameters
and a boolean return (e.g., def save_file(self, filename: str, content: str) ->
bool), open the file with encoding="utf-8" when writing, catch only IO-related
exceptions (OSError or IOError), log the error message, and return False on
failure and True on success so callers can react to the result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message lacks specificity. When catching a generic Exception, the error message only shows "Systemd Helper failed: {e}" without providing guidance on what went wrong or how to fix it. Consider catching specific exceptions (ImportError, KeyboardInterrupt, etc.) separately and providing more helpful error messages for common failure scenarios.