support workflow command

[yangbobo2021]

User description

support workflow engine.

---

Type

enhancement, bug_fix

---

Description

• Introduced new CLI options --not-store and --auto in devchat/_cli/prompt.py for controlling conversation storage and automatic function calling, along with command execution support.
• Added need_store parameter in Assistant class to control whether conversations are stored, and implemented conditional storage logic.
• Implemented CommandRunner class in devchat/engine/command_runner.py for executing commands with parameter parsing and subprocess management.
• Developed command routing and execution logic in devchat/engine/router.py, including functionality for automatic function calling.
• Improved error handling for timestamp mismatches in devchat/openai/openai_prompt.py and timestamp handling in devchat/prompt.py.

---

Changes walkthrough

	Relevant files
Enhancement	prompt.py Enhance CLI with New Options and Command Execution Support devchat/_cli/prompt.py Added sys import and run_command from devchat.engine. Introduced new CLI options --not-store and --auto for controlling conversation storage and automatic function calling. Modified the prompt function to support the new CLI options and to handle command execution with potential system exit based on command result. +21/-3    assistant.py Assistant Class Modifications for Conditional Storage        devchat/assistant.py Added need_store parameter to Assistant constructor to control conversation storage. Introduced a property getter for _prompt. Modified iterate_response to conditionally store prompts based on need_store flag. +12/-8    __init__.py Import Update to Include `run_command`                                      devchat/engine/init.py Added run_command to the list of imports from the module's components. +3/-1      command_runner.py Implement Command Execution Logic                                                devchat/engine/command_runner.py Implemented CommandRunner class to handle command execution with parameters parsing and subprocess management. +198/-0  router.py Command Routing and Execution                                                        devchat/engine/router.py Added functionality to route and execute commands based on input text and history messages. Supports loading commands, creating tools for function calling, and handling automatic function calling. +237/-0
Bug_fix	openai_prompt.py Improve Error Handling for Timestamp Mismatches                    devchat/openai/openai_prompt.py Modified error handling for timestamp mismatches in _timestamp_from_dict. +1/-2      prompt.py Timestamp Handling and Formatting Improvements                      devchat/prompt.py Added datetime import for handling timestamps. Modified formatted_header to default timestamp to current time if missing. Removed redundant formatted_header call in formatted_full_response. +3/-2

[runjinz]

PR Description updated to latest commit (5cea997)

[runjinz]

PR Review

⏱️ Estimated effort to review [1-5]	4, due to the complexity and the amount of new functionality introduced across multiple files. The PR involves enhancements and bug fixes, including new CLI options, conditional conversation storage, command execution with parameter parsing, and improved error handling. Reviewing this PR requires understanding the changes in the context of the existing codebase, assessing the impact on functionality, and ensuring that the new features work as expected without introducing new issues.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: In devchat/engine/command_runner.py, the method _call_function_by_llm attempts to parse command parameters using LLM from input text. However, if the LLM fails to generate valid parameters, the method prints an error message and returns (-1, ""). This behavior might not be handled properly in the calling context, leading to unexpected behavior or crashes.
	Performance Concern: The method run_command_with_parameters in devchat/engine/command_runner.py uses subprocess.Popen to execute commands. If not properly managed, this could lead to resource leaks or hanging processes, especially in a scenario where multiple commands are executed concurrently or in quick succession.
	Error Handling: In devchat/engine/router.py, the method _call_gpt catches exceptions and prints error messages, but it does not provide a clear mechanism for the calling code to handle these errors gracefully. This could lead to situations where errors are silently ignored or not properly logged, making debugging more difficult.
🔒 Security concerns	No

Code feedback:

---

relevant file	devchat/engine/command_runner.py
suggestion	Consider implementing a timeout for subprocesses in run_command_with_parameters to prevent hanging processes or long-running commands from consuming resources indefinitely. This can be achieved by adding a timeout parameter to subprocess.Popen and handling the TimeoutExpired exception. [important]
relevant line	with subprocess.Popen(

---

relevant file	devchat/engine/router.py
suggestion	Improve error handling in _call_gpt by either re-raising exceptions with additional context or implementing a more robust error reporting mechanism. This could include logging errors to a file or notifying an error monitoring service. This ensures that errors are not silently ignored and can be addressed more effectively. [important]
relevant line	except (ConnectionError, openai.APIConnectionError) as err:

---

relevant file	devchat/_cli/prompt.py
suggestion	Validate CLI arguments for compatibility and logical correctness. For instance, ensure that --not-store and --auto options are not mutually exclusive if that's the case. Implementing argument validation can prevent runtime errors and improve user experience. [medium]
relevant line	@click.option('-ns', '--not-store', is_flag=True, default=False, required=False,

---

relevant file	devchat/openai/openai_prompt.py
suggestion	Instead of silently updating the timestamp in _timestamp_from_dict upon mismatch, consider logging a warning or error. This would help in identifying potential issues with timestamp handling or data integrity problems. [medium]
relevant line	self._timestamp = response_data['created']

---

[runjinz]

PR Code Suggestions

Category	Suggestions
Best practice	Remove unused imports to improve code cleanliness. Consider checking if sys is actually used in this file. If it's not used, it's a good practice to remove unused imports to keep the code clean and readable. devchat/_cli/prompt.py [2] -import sys +# import sys # Removed because it's not used
	Use exceptions for error handling instead of directly exiting the program. Instead of directly exiting the program with sys.exit(command_result[0]), consider raising an exception or returning an error code to the caller. This approach provides more flexibility for error handling and makes the function more versatile in different contexts. devchat/_cli/prompt.py [112-113] if command_result is not None: - sys.exit(command_result[0]) + raise Exception(f"Command failed with exit code {command_result[0]}")
	Rename the parameter to follow boolean naming conventions. The need_store parameter in the Assistant class constructor should be renamed to should_store to better reflect its purpose as a boolean flag and align with common naming conventions for boolean variables. devchat/assistant.py [16] -def __init__(self, chat: Chat, store: Store, max_prompt_tokens: int, need_store: bool): +def __init__(self, chat: Chat, store: Store, max_prompt_tokens: int, should_store: bool):
Security	Sanitize command parameters to prevent command injection. The run_command_with_parameters method uses a subprocess to execute commands. It's important to validate and sanitize the parameters to avoid command injection vulnerabilities, especially since these parameters can come from user input. devchat/engine/command_runner.py [159-162] command_run = command.steps[0]["run"] -# Replace parameters in command run +# Replace parameters in command run with sanitized values for parameter in parameters: - command_run = command_run.replace('$' + parameter, str(parameters[parameter])) + sanitized_value = sanitize(parameters[parameter]) # Implement sanitize function + command_run = command_run.replace('$' + parameter, sanitized_value)
Enhancement	Handle missing or invalid directories more gracefully. When loading commands with _load_command and _load_commands, it's a good practice to handle the case where the directory does not exist or is not a directory more gracefully. Instead of returning None, consider logging an error or raising an exception to inform the user or caller about the issue. devchat/engine/router.py [14-17] if not os.path.exists(workflows_dir): - return None + raise FileNotFoundError(f"Workflows directory {workflows_dir} does not exist.") if not os.path.isdir(workflows_dir): - return None + raise NotADirectoryError(f"{workflows_dir} is not a directory.")
	Use timezone-aware datetime for generating timestamps. Instead of directly using datetime.timestamp(datetime.now()), consider using a more precise and timezone-aware approach by specifying the timezone, for consistency and accuracy across different environments. devchat/prompt.py [228] -self._timestamp = datetime.timestamp(datetime.now()) +from datetime import timezone +self._timestamp = datetime.now(timezone.utc).timestamp()
	Handle cases where formatted_str might remain empty. Initializing formatted_str to an empty string and then conditionally appending content might lead to scenarios where formatted_str remains empty. Consider adding a default message or handling the case where no content is appended. devchat/prompt.py [271] -formatted_str = "" +formatted_str = "No content available."
Possible issue	Add validation for 'created' key in response_data to prevent runtime errors. Consider handling the case where response_data['created'] is not present or is invalid. This could prevent runtime errors if response_data does not contain a 'created' key or if its value is not in the expected format. devchat/openai/openai_prompt.py [242] -self._timestamp = response_data['created'] +if 'created' in response_data and isinstance(response_data['created'], (int, float)): + self._timestamp = response_data['created'] +else: + raise KeyError("Expected 'created' key with a valid timestamp in response_data")

[runjinz]

Preparing PR description...