feat: add interactive troubleshooting assistant #584
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
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. 📝 WalkthroughWalkthroughAdds an interactive AI-driven Troubleshooter CLI with optional sandboxed command execution, a JSON-backed ResolutionManager for persisting/searching fixes, new Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant TS as "Troubleshooter"
participant AI as "AI Agent"
participant Shell as "Shell/System"
participant RM as "ResolutionManager"
User->>TS: start()
TS->>TS: detect API key / provider
TS->>AI: init with system prompt + memory
AI-->>TS: initial analysis
loop interactive session
TS->>RM: search(current_problem)
RM-->>TS: historical resolutions
TS->>AI: send problem + memory
AI-->>TS: analysis + code block(s)
TS->>TS: extract code blocks
alt command safe
TS->>Shell: execute command (optional Firejail)
Shell-->>TS: stdout/stderr
TS->>AI: send execution results
else command dangerous
TS-->>User: block & request confirmation
User->>TS: confirm / decline
end
User->>TS: continue / doctor / help / quit
end
alt problem solved
TS->>TS: synthesize issue+fix
TS->>RM: save(issue, fix)
RM-->>TS: persisted
end
TS-->>User: exit(code)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Summary of ChangesHello @KrishnaShuk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates an advanced AI-powered troubleshooting assistant into Cortex. The primary goal is to streamline the process of identifying and resolving system problems by allowing an AI to interactively guide users, suggest solutions, and safely execute commands, thereby enhancing user experience and system maintainability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an interactive troubleshooting assistant, which is a great feature. The implementation is solid, leveraging rich for a good user experience and including important safety features like a dangerous command blacklist and user confirmation before execution.
My review focuses on enhancing security and improving code efficiency. I've identified a potential security gap in the command blacklist and suggest adding patterns to prevent command execution via pipes. I've also pointed out an opportunity to refactor the AI initialization logic for better performance and clarity, and noted an unused import.
Overall, this is a well-executed feature addition. Addressing these points will make it even more robust and secure.
KrishnaShuk
changed the title
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
AI coding agent was used for framing better tests and creating the command file.
@KrishnaShuk But which one ?
Also, missing demonstrating video and documentation file.
|
This PR is still in the draft. I will complete the details soon. |
CLA Verification PassedAll contributors have signed the CLA.
|
|
PTAL @Anshgrover23! |
|
@coderabbitai review. |
|
@Anshgrover23: I'll review the PR now. Let me analyze the changes and provide feedback on the implementation of the interactive troubleshooting assistant. ✅ Actions performedReview triggered.
|
|
@Anshgrover23: I'll review the changes in this PR now. ✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @cortex/troubleshoot.py:
- Around line 207-214: The analysis call to self.ai.ask in Troubleshooter (the
block that sets analysis = self.ai.ask(...)) omits the troubleshooter's system
prompt, causing AskHandler to use its default persona; update that call to
include the same system prompt used by the troubleshooter (e.g., pass
system_prompt=self.system_prompt or the variable name used elsewhere in the
class) so the follow-up analysis uses the same persona/context as prior
messages.
🧹 Nitpick comments (7)
cortex/troubleshoot.py (4)
70-80: Unused helper methods_get_providerand_get_api_key.These methods duplicate logic already executed in
__init__and are never called anywhere. Consider removing them to reduce dead code.🔧 Remove unused methods
- def _get_provider(self) -> str: - """Determine which LLM provider to use.""" - found, _, provider, _ = auto_detect_api_key() - if provider == "anthropic": - return "claude" - return provider or "openai" - - def _get_api_key(self) -> str: - """Get the API key for the configured provider.""" - found, key, _, _ = auto_detect_api_key() - return key or ""
28-44: Consider expanding dangerous command patterns.The current blacklist is a good start but could be enhanced:
- Pattern on line 31 (
r"\brm\s+(-[^\s]*\s+)*/\s*$") requires the command to end with/butrm -rf / --no-preserve-rootwould bypass it since it doesn't end with/.- Consider adding patterns for
curl ... | bash,wget -O- ... | sh, andevalwith untrusted input.🔒 Suggested additional patterns
DANGEROUS_PATTERNS = [ r"\brm\s+(-[^\s]*\s+)*-rf\b", # rm -rf r"\brm\s+(-[^\s]*\s+)*-fr\b", # rm -fr (same as above) - r"\brm\s+(-[^\s]*\s+)*/\s*$", # rm / + r"\brm\s+(-[^\s]*\s+)*/", # rm / (anywhere in command) r"\bmkfs\b", # Format filesystem r"\bdd\s+.*of=/dev/", # dd to device r">\s*/dev/sd[a-z]", # Redirect to disk r"\bchmod\s+(-[^\s]*\s+)*777\s+/", # chmod 777 on root r"\bchown\s+.*\s+/\s*$", # chown on root r":\(\)\s*{\s*:\|:\s*&\s*}", # Fork bomb r"\bshutdown\b", # Shutdown r"\breboot\b", # Reboot r"\binit\s+0\b", # Halt r"\bpoweroff\b", # Poweroff r"\|\s*bash", # Pipe to bash r"\|\s*sh", # Pipe to sh + r"\beval\s+", # eval command + r"\bcurl\b.*\|\s*sudo", # curl piped to sudo + r"\bwget\b.*\|\s*sudo", # wget piped to sudo ]
13-13: Unused importOptional.The code uses Python 3.10+ union syntax (
str | None) throughout, making theOptionalimport unnecessary.♻️ Remove unused import
-from typing import Optional
47-68: Add class-level docstring forTroubleshooter.Per coding guidelines, public APIs should have docstrings. The class is missing documentation describing its purpose and usage.
📝 Add docstring
class Troubleshooter: + """Interactive AI-powered troubleshooting assistant. + + Provides a conversational interface for diagnosing and resolving + system issues. Suggests shell commands, executes them with user + confirmation, and analyzes output to guide next steps. + + Attributes: + logger: CortexLogger instance for logging operations. + messages: Conversation history for context. + ai: AskHandler instance for LLM interactions. + provider: The LLM provider being used. + api_key: API key for the provider. + """ def __init__(self):docs/COMMANDS.md (2)
157-177: Add language specifier to fenced code block.The flow diagram code block lacks a language specifier. While it's ASCII art, adding a specifier like
textor leaving it empty is fine, but markdown linters flag this.📝 Fix markdown lint warning
**Flow:** -``` +```text ┌─────────────────────────────────────────┐ │ User describes issue │
179-199: Add language specifier to example session code block end.The code block ending at line 199 lacks a language specifier for the closing context after the bash block.
Note: The nested bash block structure makes this tricky. Consider restructuring to avoid the lint warning or accept it as-is since the content is illustrative.
tests/test_troubleshoot.py (1)
324-341: Test for doctor command doesn't verify invocation.The comment on line 340-341 acknowledges that
SystemDoctoris imported inside the function, making it difficult to verify the call. Consider refactoring to enable proper verification.💡 Alternative approach to verify doctor invocation
@patch("cortex.troubleshoot.console") @patch("cortex.troubleshoot.Prompt") def test_doctor_command(self, mock_prompt, mock_console): """Test that 'doctor' command runs SystemDoctor.""" mock_prompt.ask.side_effect = ["doctor", "exit"] with patch("cortex.troubleshoot.auto_detect_api_key") as mock_detect: mock_detect.return_value = (True, "test-key", "fake", "env") with patch("cortex.ask.AskHandler"): # Patch at the point of import inside the function with patch.dict('sys.modules', {'cortex.doctor': MagicMock()}) as mock_doctor_module: mock_doctor_instance = MagicMock() mock_doctor_module.SystemDoctor.return_value = mock_doctor_instance troubleshooter = Troubleshooter() troubleshooter.messages = [{"role": "system", "content": "test"}] troubleshooter._interactive_loop() # Now we can verify mock_doctor_instance.run_checks.assert_called_once()
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cortex/ask.pycortex/cli.pycortex/troubleshoot.pydocs/COMMANDS.mdtests/test_troubleshoot.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide for Python code
Include type hints in Python code
Add docstrings for all public APIs in Python code
Use dry-run mode by default for all installation operations
Do not use silent sudo - require explicit user confirmation for privilege escalation
Implement Firejail sandboxing for execution of untrusted code
Log all operations to ~/.cortex/history.db for audit purposes
Files:
cortex/troubleshoot.pycortex/ask.pytests/test_troubleshoot.pycortex/cli.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain test coverage above 80% for pull requests
Files:
tests/test_troubleshoot.py
🧬 Code graph analysis (4)
cortex/troubleshoot.py (3)
cortex/api_key_detector.py (1)
auto_detect_api_key(657-665)cortex/ask.py (1)
AskHandler(137-354)cortex/cli.py (1)
doctor(2004-2009)
cortex/ask.py (2)
cortex/cli.py (1)
ask(604-633)cortex/llm/interpreter.py (1)
_get_system_prompt(109-143)
tests/test_troubleshoot.py (2)
cortex/cli.py (1)
troubleshoot(2011-2016)cortex/troubleshoot.py (6)
Troubleshooter(47-224)_extract_code_blocks(106-110)_is_command_safe(112-121)_execute_command(123-132)start(82-104)_interactive_loop(134-224)
cortex/cli.py (2)
cortex/doctor.py (2)
SystemDoctor(21-478)run_checks(48-103)cortex/troubleshoot.py (1)
Troubleshooter(47-224)
🪛 markdownlint-cli2 (0.18.1)
docs/COMMANDS.md
157-157: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (13)
cortex/ask.py (1)
288-310: LGTM! Clean backward-compatible API extension.The optional
system_promptparameter allows external callers (like the newTroubleshooter) to supply a custom prompt while preserving the original behavior when omitted. The conditional logic correctly skips context gathering when a prompt is provided.cortex/cli.py (3)
2004-2016: LGTM! Clean implementation following existing patterns.The new
doctor()andtroubleshoot()methods correctly use lazy imports for faster CLI startup and follow the established pattern of delegating to domain-specific classes.
2495-2499: LGTM! Subparser definitions are correct.The new subparsers for
doctorandtroubleshootare properly defined and will integrate with the existing CLI infrastructure.
2554-2557: LGTM! Command routing is correct.The new commands are properly routed to their respective CLI methods.
docs/COMMANDS.md (2)
14-15: LGTM! Quick reference table updated correctly.The new commands are properly documented in the quick reference.
140-217: Excellent documentation for the troubleshoot feature.The documentation is thorough with clear examples, flow diagrams, safety information, and special commands. This will help users understand and use the feature effectively.
tests/test_troubleshoot.py (7)
1-11: LGTM! Test file setup is correct.Proper imports and path setup for test discovery.
13-88: Excellent test coverage for_extract_code_blocks.Tests cover bash blocks, sh blocks, generic blocks, multiple blocks, no blocks, and multiline commands. This thoroughly validates the regex extraction logic.
90-163: Good coverage for_is_command_safeblacklist enforcement.Tests validate both safe commands (ls, df, systemctl) and dangerous commands (rm -rf, mkfs, dd, shutdown, reboot, chmod 777 /). Consider adding edge case tests for bypass attempts.
165-198: Good_execute_commandtests including timeout handling.Tests cover simple commands, stderr capture, and timeout exceptions. The mocking approach for timeout is correct.
371-397: Excellent integration test for command execution flow.This test validates the full happy path: user input → AI response with code block → user confirmation → command execution. Good use of
side_effectfor sequencing mock responses.
399-420: Critical safety test: dangerous commands are blocked.Good test verifying that
rm -rf /is blocked and_execute_commandis never called. This is essential for the security guarantee.
478-479: LGTM! Test file is well-structured.The test suite provides good coverage for the Troubleshooter component with proper isolation through mocking.
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@KrishnaShuk Issue requirement is incompleted:
- Learn from successful resolutions
- Escalation path to human support
Also, kindly address bot comments as well.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cortex/troubleshoot.py`:
- Around line 55-56: Add a class-level docstring to the public Troubleshooter
class describing its purpose and public behavior, and annotate the __init__
method with an explicit return type -> None; update the class definition for
Troubleshooter and the __init__ signature (method name __init__) accordingly so
the public API has proper documentation and typing.
- Around line 34-38: DANGEROUS_PATTERNS currently matches joined flags like
"-rf" but not separated flags; update the DANGEROUS_PATTERNS list in
cortex/troubleshoot.py to include a regex that detects any "rm" invocation
containing both recursive and force flags in any order (e.g., "-r" and "-f"
possibly separated by other flags/whitespace) so commands like "rm -r -f /" or
"rm -f -r file" are caught; add the new pattern alongside the existing "rm
-rf"/"rm -fr" entries and ensure it anchors on the word "rm" and allows other
flags between "-r" and "-f".
♻️ Duplicate comments (1)
cortex/troubleshoot.py (1)
127-155: Add history.db audit logging for executed commands.Logging to
CortexLoggerisn’t the same as the required audit trail in~/.cortex/history.db. Please record each executed command (timestamp, cmd, exit code, and a truncated output) into the history DB—ideally via the existing history/audit layer.As per coding guidelines, operations must be logged to
~/.cortex/history.dbfor auditability.
|
@Anshgrover23 PTAL! |
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@KrishnaShuk Add updated video in the PR Description.
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@KrishnaShuk Kindly resolve conflicts.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cortex/troubleshoot.py`:
- Around line 136-138: The code logs raw command strings via
self.logger.info(f"Executing command: {cmd}"), risking secret leakage; create
and call a sanitizer (e.g., redact_sensitive_flags or sanitize_command) that
strips or replaces values for common secret flags/keys (examples: --token,
--access-token, --password, -p, --secret, --key, --auth) by handling both
"--flag=value" and "--flag value" patterns with regex, then replace the logger
call to log the sanitized_cmd (or alternatively log only the command name and
arg count) so no secret values are emitted.
🧹 Nitpick comments (1)
cortex/troubleshoot.py (1)
171-173: Filter out system prompt when building session history.Including the system prompt in the history payload adds noise and can bleed internal instructions into summaries/extraction. Consider a small helper that omits
role == "system"and reuse it for both the learning and help flows.♻️ Suggested helper for history formatting
+ def _format_history(self, include_system: bool = False) -> str: + messages = [ + m for m in self.messages if include_system or m["role"] != "system" + ] + return "\n".join(f"{m['role']}: {m['content']}" for m in messages) + @@ - history_text = "\n".join( - [f"{m['role']}: {m['content']}" for m in self.messages] - ) + history_text = self._format_history() @@ - history_text = "\n".join( - [f"{m['role']}: {m['content']}" for m in self.messages] - ) + history_text = self._format_history()
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cortex/troubleshoot.py`:
- Around line 230-235: The file writes the troubleshooting support log using
open(log_file, "w") which relies on the platform default encoding and can raise
UnicodeEncodeError for AI-generated summaries; update the log-writing call in
cortex/troubleshoot.py to open the file with an explicit UTF-8 encoding (e.g.,
use encoding="utf-8") when opening log_file in the try block that writes the
"Cortex Troubleshooting Log" and the variable summary so all Unicode characters
are handled consistently.
♻️ Duplicate comments (1)
cortex/troubleshoot.py (1)
136-138: Redact secrets before logging commands.
Raw command strings may include tokens or passwords; logging them verbatim risks credential leakage.🔒 Proposed fix: sanitize before logging
+ def _sanitize_command_for_log(self, cmd: str) -> str: + """Redact common secret flags from command strings before logging.""" + return re.sub( + r"(?i)(--?(?:token|api[-_]?key|access[-_]?token|password|secret|auth)(?:=|\s+))(\S+)", + r"\1***", + cmd, + ) + def _execute_command(self, cmd: str) -> str: """Execute a shell command and return output. @@ - self.logger.info(f"Executing command: {cmd}") + redacted_cmd = self._sanitize_command_for_log(cmd) + self.logger.info(f"Executing command: {redacted_cmd}")- self.logger.warning(f"Blocked dangerous command: {cmd}") + self.logger.warning( + f"Blocked dangerous command: {self._sanitize_command_for_log(cmd)}" + )Also applies to: 298-299
Anshgrover23
left a comment
Anshgrover23
left a comment
There was a problem hiding this comment.
@KrishnaShuk The help command (for escalation) is missing from the Special Commands table. Add it alongside doctor, exit, etc.Otherwise LGTM 👍
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@cortex/troubleshoot.py`:
- Around line 34-50: The DANGEROUS_PATTERNS list currently misses
command-substitution and escaping bypasses; update DANGEROUS_PATTERNS to add
regexes that detect backtick substitution (`...`) and $() substitution when
adjacent to dangerous commands (e.g., patterns that match
\b(?:rm|dd|mkfs|chmod|chown|shutdown|reboot|poweroff|init)\b\s*(?:`[^`]*`|\$\([^)]*\)
) ), and add patterns to catch escaped characters breaking word boundaries
(e.g., matches like r\\m or other interspersed backslashes using something like
r"(?:r\\+m|d\\+d|m\\+kfs)" or a more general pattern that treats backslashes
between letters as still forming the command); modify the DANGEROUS_PATTERNS
constant accordingly so the new regexes are evaluated alongside existing ones.
- Around line 235-240: Update the test assertion for the mocked open call in the
troubleshoot tests so it matches the actual call in the code: include the
encoding keyword argument "utf-8" in the mock_file.assert_called_with
invocation; specifically ensure the test's mock_file.assert_called_with(...)
includes the same filename, mode "w", and encoding="utf-8" to match the
open(log_file, "w", encoding="utf-8") call in troubleshoot.py.
- Around line 169-171: The Prompt.ask loop allows unbounded user input; add a
length validation step after reading user_input from Prompt.ask in the
interactive loop (where Prompt.ask is called) to prevent huge messages from
being appended to self.messages and sent to the LLM. Specifically, check
len(user_input) (or byte length if needed) and either reject with a clear prompt
to the user or truncate to a configured MAX_INPUT_LENGTH constant, then only
append the validated/truncated string to self.messages and proceed; update any
code that sends self.messages to the LLM to rely on this validation.
🧹 Nitpick comments (1)
cortex/troubleshoot.py (1)
113-117: Consider handling Windows line endings in code block extraction.The pattern
r"```(?:bash|sh)?\n(.*?)```"requires\nafter the language identifier. LLM responses typically follow this format, but if responses contain Windows line endings (\r\n), they won't match.♻️ More robust pattern (optional)
def _extract_code_blocks(self, text: str) -> list[str]: """Extract content from markdown code blocks.""" # Match ```bash ... ``` or ```sh ... ``` or just ``` ... ``` - pattern = r"```(?:bash|sh)?\n(.*?)```" + pattern = r"```(?:bash|sh)?\r?\n(.*?)```" return re.findall(pattern, text, re.DOTALL)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/COMMANDS.md`:
- Around line 156-200: The Flow and Example Session code blocks in COMMANDS.md
are missing language identifiers and contain a nested fenced block that breaks
Markdown; update the Flow and Example Session blocks to use a single top-level
fenced block with a language (e.g., ```text) and remove the inner
triple-backtick fence by inlining the command (replace the nested ```bash block
with a plain indented or inline "$ systemctl status docker" line); specifically
edit the "Flow" and "Example Session" sections to use ```text fences and ensure
there are no nested fences so the markdown renders correctly.
♻️ Duplicate comments (1)
cortex/troubleshoot.py (1)
133-167: Redaction misses--flag=valueand short flag secrets.Current sanitization only catches
--flag value. Secrets passed as--token=...or-p ...can still leak to logs.🔒 Suggested redaction hardening
- redacted = re.sub( - r"(?i)(--?(?:token|api[-_]?key|password|secret)\s+)(\S+)", - r"\1***", - cmd, - ) + redacted = re.sub( + r"(?i)(--?(?:token|api[-_]?key|password|secret|access[-_]?token|auth|key)|-p)(\s+|=)(\S+)", + r"\1\2***", + cmd, + )
🧹 Nitpick comments (5)
tests/test_troubleshoot.py (3)
96-175: Add safety tests for backticks and$()patterns.You already added regex patterns for command substitution; please add explicit tests so regressions are caught.
✅ Suggested test additions
def test_safe_chmod_normal(self): """Test that 'chmod 755' on a normal directory is safe.""" is_safe, reason = self.troubleshooter._is_command_safe("chmod 755 /tmp/mydir") self.assertTrue(is_safe) + + def test_dangerous_backticks(self): + """Test that backtick substitution is blocked.""" + is_safe, _ = self.troubleshooter._is_command_safe("echo `rm -rf /`") + self.assertFalse(is_safe) + + def test_dangerous_subshell(self): + """Test that $() substitution is blocked.""" + is_safe, _ = self.troubleshooter._is_command_safe("rm $(echo -rf /)") + self.assertFalse(is_safe)
177-253: Make execution tests deterministic w.r.t. Firejail.
test_execute_simple_commandandtest_execute_command_with_stderrcurrently depend on whether Firejail is installed. Patchshutil.whichto keep CI stable.♻️ Suggested patching
- def test_execute_simple_command(self): + `@patch`("cortex.troubleshoot.shutil.which", return_value=None) + def test_execute_simple_command(self, _mock_which): """Test executing a simple echo command.""" output = self.troubleshooter._execute_command("echo 'hello world'") self.assertIn("hello world", output) - def test_execute_command_with_stderr(self): + `@patch`("cortex.troubleshoot.shutil.which", return_value=None) + def test_execute_command_with_stderr(self, _mock_which): """Test command that produces stderr.""" output = self.troubleshooter._execute_command("ls /nonexistent_directory_12345") self.assertIn("[STDERR]", output)
371-681: Assert the doctor command is actually invoked.You already patch
cortex.doctor.SystemDoctor, so you can assert the call rather than skipping it.✅ Suggested assertion
troubleshooter.messages = [{"role": "system", "content": "test"}] troubleshooter._interactive_loop() - # The doctor command should be called via the import inside the loop - # Since SystemDoctor is imported inside the function, we skip this assertion + mock_doctor_instance.run_checks.assert_called_once()cortex/resolutions.py (2)
25-28: Add class docstring +__init__return type for public API.
ResolutionManageris public and should have a class docstring;__init__should be annotated.📌 Suggested update
-class ResolutionManager: - def __init__(self, storage_path: str = "~/.cortex/resolutions.json"): +class ResolutionManager: + """Persist and query troubleshooting resolutions.""" + + def __init__(self, storage_path: str = "~/.cortex/resolutions.json") -> None:As per coding guidelines, public APIs should include docstrings and type hints.
70-100: Consider a shared lock during reads.Without a shared lock,
searchcan read the file while a writer truncates it, yielding transient empty results. A shared lock reduces that window.🔒 Suggested shared lock
- try: - with open(self.storage_path) as f: - resolutions: list[Resolution] = json.load(f) + try: + with open(self.storage_path) as f: + fcntl.flock(f, fcntl.LOCK_SH) + try: + resolutions: list[Resolution] = json.load(f) + finally: + fcntl.flock(f, fcntl.LOCK_UN) except (json.JSONDecodeError, FileNotFoundError): return []
|
|
Hey @Anshgrover23, I have resolved all recommendations and the PR is passing all the tests. |
2 participants
Related Issue
Closes #59
Summary
Adds an interactive AI-powered troubleshooting assistant (cortex troubleshoot) that can diagnose system issues, suggest shell commands, and execute them on behalf of the user with explicit confirmation. Includes a comprehensive dangerous command blacklist to prevent accidental execution of destructive commands like rm -rf.
AI Disclosure
Demonstration
Screencast.from.2026-01-18.00-42-52.webm
cortex_support_log.txt: https://gist.github.com/KrishnaShuk/4d528dd82bf5dcdd022562457d8b23fb
resolutions.json[for learning purpose]: https://gist.github.com/KrishnaShuk/a41e040bbd9290454b7a15c1017faee9
Google antigravity ai coding assistant(Claude Opus 4.5 ) was used for framing better tests and creating the command file.
Checklist
type(scope): descriptionor[scope] descriptionpytest tests/)Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.