Fix review comments: API key storage, file operations, validation

- Update documentation to clarify API keys stored in plain text
- Implement _apply_task_changes to actually write edited/created files
- Remove unused imports from copilot_panel.py
- Add validation for audit comments (require non-empty)
- Add error handling for parent.audit_task calls
- Add temperature validation (0-2 range)
- Implement stop parameter in text_completion
- Add configurable constants for max_tokens and context sizes
- Store thread references to prevent premature GC
- Use single-shot connections for callbacks
- Fix race condition in signal connections
- Add _cleanup_thread method for proper thread management

Co-authored-by: HeDaas-Code <208586641+HeDaas-Code@users.noreply.github.com>

Author: Copilot

docs/copilot_config.md

@@ -56,10 +56,10 @@ The recommended way to configure Copilot is through the UI:
 
 ## Security Notes
 
-- API keys are stored in local configuration file
+- API keys are stored in plain text in local configuration file
+- Ensure proper file system permissions to protect configuration files
 - Never commit configuration files with API keys to git
 - Use environment variables for CI/CD if needed
-- Keys are encrypted in configuration storage
 
 ## Environment Variables (Optional)
 

docs/copilot_guide.md

@@ -222,7 +222,7 @@ MGit Copilot 是一个专注于写作的AI助手，集成了最新的大语言
 ### Q: 如何保护隐私？
 
 **A**:
-1. API密钥加密存储在本地配置文件
+1. API密钥以明文存储在本地配置文件，请确保操作系统账户和配置文件的访问权限安全
 2. 不要在提示词中包含敏感信息
 3. 审计模式可以预览所有修改
 4. 可以随时禁用Copilot功能

src/components/copilot_panel.py

@@ -7,13 +7,12 @@
 
 from PyQt5.QtWidgets import (QWidget, QVBoxLayout, QHBoxLayout, QLabel, 
                            QTextEdit, QLineEdit, QPushButton, QComboBox,
-                           QGroupBox, QListWidget, QListWidgetItem, QSplitter,
-                           QTabWidget, QFrame, QMessageBox, QDialog)
-from PyQt5.QtCore import Qt, pyqtSignal, QTimer
-from PyQt5.QtGui import QFont, QColor, QTextCursor
+                           QListWidget, QListWidgetItem,
+                           QTabWidget, QMessageBox, QDialog)
+from PyQt5.QtCore import Qt, pyqtSignal
+from PyQt5.QtGui import QTextCursor
 from qfluentwidgets import (PushButton, LineEdit, ComboBox, TextEdit, 
-                           CardWidget, SubtitleLabel, BodyLabel, 
-                           FluentIcon, IconWidget)
+                           SubtitleLabel, BodyLabel)
 from src.utils.logger import info, warning, error
 from src.copilot.agent_mode import AgentTask
 from datetime import datetime
@@ -327,8 +326,25 @@ def _show_audit_dialog(self, task_id: str):
         dialog = TaskAuditDialog(task_id, self)
         if dialog.exec_():
             approved, comment = dialog.get_result()
-            # Signal parent to handle audit
-            self.parent().audit_task(task_id, approved, comment)
+            # Signal parent to handle audit with error handling
+            parent = self.parent()
+            if parent is None or not hasattr(parent, "audit_task") or not callable(getattr(parent, "audit_task")):
+                warning(f"CopilotPanel parent has no 'audit_task' method; cannot audit task {task_id}")
+                QMessageBox.warning(
+                    self,
+                    "审计失败",
+                    "无法处理审计请求：未找到上级处理器。"
+                )
+                return
+            try:
+                parent.audit_task(task_id, approved, comment)
+            except Exception as e:
+                error(f"Error while auditing task {task_id}: {e}")
+                QMessageBox.critical(
+                    self,
+                    "审计错误",
+                    "处理审计请求时发生错误，请查看日志。"
+                )
 
 
 class TaskAuditDialog(QDialog):
@@ -377,14 +393,22 @@ def initUI(self):
 
     def _on_approve(self):
         """Handle approve"""
+        comment_text = self.comment_edit.toPlainText().strip()
+        if not comment_text:
+            QMessageBox.warning(self, "输入必需", "请填写审计意见后再继续。")
+            return
         self.approved = True
-        self.comment = self.comment_edit.toPlainText()
+        self.comment = comment_text
         self.accept()
 
     def _on_reject(self):
         """Handle reject"""
+        comment_text = self.comment_edit.toPlainText().strip()
+        if not comment_text:
+            QMessageBox.warning(self, "输入必需", "请填写审计意见后再继续。")
+            return
         self.approved = False
-        self.comment = self.comment_edit.toPlainText()
+        self.comment = comment_text
         self.accept()
 
     def get_result(self):

src/copilot/agent_mode.py

@@ -292,8 +292,31 @@ def audit_task(self, task_id: str, approved: bool, comment: str = ""):
     def _apply_task_changes(self, task: AgentTask):
         """Apply approved task changes"""
         if task.task_type in ['edit', 'create']:
-            # Changes already in result, application should handle
-            pass
+            # Write the edited/created content to file system
+            if not task.result or not isinstance(task.result, dict):
+                warning(f"No valid result to apply for task {task.task_id}")
+                return
+                
+            file_path = task.result.get('file_path')
+            content = task.result.get('edited_content') or task.result.get('content')
+            
+            if not file_path or content is None:
+                warning(f"Incomplete task result for {task.task_id}: path={file_path}, has_content={content is not None}")
+                return
+                
+            try:
+                # Create directory if needed
+                directory = os.path.dirname(file_path)
+                if directory:
+                    os.makedirs(directory, exist_ok=True)
+                
+                # Write content to file
+                with open(file_path, 'w', encoding='utf-8') as f:
+                    f.write(content)
+                    
+                info(f"Applied changes to {file_path} for task {task.task_id}")
+            except Exception as e:
+                error(f"Failed to apply changes for task {task.task_id}: {str(e)}")
         elif task.task_type == 'commit':
             # Commit was already made during execution
             pass

src/copilot/copilot_manager.py

@@ -6,11 +6,19 @@
 """
 
 from typing import Dict, List, Optional, Callable
-from PyQt5.QtCore import QObject, pyqtSignal, QThread
+from PyQt5.QtCore import QObject, pyqtSignal, QThread, Qt
 from .siliconflow_client import SiliconFlowClient
 from src.utils.logger import info, warning, error
 from src.utils.config_manager import ConfigManager
 
+# Configuration constants
+MAX_TOKENS_COMPLETION = 500
+MAX_TOKENS_EDIT = 2048
+MAX_TOKENS_CREATION = 4096
+MAX_TOKENS_CHAT = 2048
+MAX_CONTEXT_BEFORE = 500  # Characters of context before cursor
+MAX_CONTEXT_AFTER = 100   # Characters of context after cursor
+
 class CopilotManager(QObject):
     """
     Manager for copilot functionality
@@ -29,6 +37,7 @@ def __init__(self, config_manager: ConfigManager):
         self.client = None
         self.enabled = False
         self.current_mode = 'none'  # none, inline, edit, creation, conversation, agent
+        self.current_threads = []  # Store active threads
 
         # Load configuration
         self._load_config()
@@ -112,9 +121,13 @@ def get_inline_completion(
         thread.completion_ready.connect(self._on_completion_ready)
         thread.error_occurred.connect(self._on_error)
 
+        # Store thread reference and connect callback if provided
+        self.current_threads.append(thread)
         if callback:
-            thread.completion_ready.connect(callback)
+            # Use a one-shot connection
+            thread.completion_ready.connect(callback, Qt.SingleShotConnection)
 
+        thread.finished.connect(lambda: self._cleanup_thread(thread))
         thread.start()
 
     def edit_text(
@@ -239,6 +252,11 @@ def _on_error(self, error_msg: str):
         self.status_changed.emit("Error occurred")
         self.current_mode = 'none'
         error(f"Copilot error: {error_msg}")
+        
+    def _cleanup_thread(self, thread: QThread):
+        """Remove finished thread from active threads list"""
+        if thread in self.current_threads:
+            self.current_threads.remove(thread)
 
 
 class CompletionThread(QThread):
@@ -267,7 +285,7 @@ def run(self):
 请生成自然的补全内容（只返回补全内容，不要包含任何解释）："""
 
             messages = [{'role': 'user', 'content': prompt}]
-            response = self.client.chat_completion(messages, temperature=0.7, max_tokens=500)
+            response = self.client.chat_completion(messages, temperature=0.7, max_tokens=MAX_TOKENS_COMPLETION)
 
             if 'choices' in response and len(response['choices']) > 0:
                 completion = response['choices'][0]['message']['content'].strip()
@@ -303,7 +321,7 @@ def run(self):
 请返回编辑后的文本（只返回编辑后的文本，不要包含任何解释）："""
 
             messages = [{'role': 'user', 'content': prompt}]
-            response = self.client.chat_completion(messages, temperature=0.5, max_tokens=2048)
+            response = self.client.chat_completion(messages, temperature=0.5, max_tokens=MAX_TOKENS_EDIT)
 
             if 'choices' in response and len(response['choices']) > 0:
                 edited = response['choices'][0]['message']['content'].strip()
@@ -336,7 +354,7 @@ def run(self):
                 {'role': 'system', 'content': system_prompt},
                 {'role': 'user', 'content': self.prompt}
             ]
-            response = self.client.chat_completion(messages, temperature=0.8, max_tokens=4096)
+            response = self.client.chat_completion(messages, temperature=0.8, max_tokens=MAX_TOKENS_CREATION)
 
             if 'choices' in response and len(response['choices']) > 0:
                 content = response['choices'][0]['message']['content'].strip()
@@ -365,7 +383,7 @@ def run(self):
             messages = self.history.copy()
             messages.append({'role': 'user', 'content': self.message})
 
-            response = self.client.chat_completion(messages, temperature=0.7, max_tokens=2048)
+            response = self.client.chat_completion(messages, temperature=0.7, max_tokens=MAX_TOKENS_CHAT)
 
             if 'choices' in response and len(response['choices']) > 0:
                 reply = response['choices'][0]['message']['content'].strip()

src/copilot/siliconflow_client.py

@@ -49,13 +49,18 @@ def chat_completion(
         
         Args:
             messages: List of message dicts with 'role' and 'content'
-            temperature: Sampling temperature (0-1)
+            temperature: Sampling temperature (0-2)
             max_tokens: Maximum tokens to generate
             stream: Whether to stream the response
             
         Returns:
             Response dict from API
         """
+        # Validate temperature
+        if not 0 <= temperature <= 2:
+            warning(f"Temperature {temperature} out of range [0, 2], clamping")
+            temperature = max(0, min(2, temperature))
+            
         url = f"{self.BASE_URL}/chat/completions"
         data = {
             'model': self.model,
@@ -139,7 +144,21 @@ def text_completion(
         response = self.chat_completion(messages, temperature, max_tokens)
 
         if 'choices' in response and len(response['choices']) > 0:
-            return response['choices'][0]['message']['content']
+            content = response['choices'][0]['message']['content']
+            
+            # Apply stop sequences if provided
+            if stop:
+                first_stop_index = None
+                for s in stop:
+                    if not s:
+                        continue
+                    idx = content.find(s)
+                    if idx != -1 and (first_stop_index is None or idx < first_stop_index):
+                        first_stop_index = idx
+                if first_stop_index is not None:
+                    content = content[:first_stop_index]
+                    
+            return content
         return ""
 
     def get_embedding(self, text: str) -> List[float]:

src/views/main_window.py

@@ -1317,40 +1317,46 @@ def toggleCopilotPanel(self):
 
         if not is_visible:
             info("Copilot面板已显示")
-            # 连接copilot信号
-            self._connect_copilot_signals()
+            # 连接copilot信号 (with lock to prevent race condition)
+            if not hasattr(self, '_copilot_signals_lock'):
+                self._copilot_signals_lock = False
+            if not self._copilot_signals_lock:
+                self._copilot_signals_lock = True
+                self._connect_copilot_signals()
         else:
             info("Copilot面板已隐藏")
 
     def _connect_copilot_signals(self):
         """连接Copilot面板信号"""
-        if not hasattr(self, '_copilot_signals_connected'):
-            self.copilotPanel.completion_requested.connect(
-                lambda before, after: self.copilotManager.get_inline_completion(
-                    before, after, self._on_completion_ready
-                )
-            )
-            self.copilotPanel.edit_requested.connect(
-                lambda text, instruction: self.copilotManager.edit_text(
-                    text, instruction, self._on_edit_ready
-                )
+        if hasattr(self, '_copilot_signals_connected') and self._copilot_signals_connected:
+            return  # Already connected
+            
+        self.copilotPanel.completion_requested.connect(
+            lambda before, after: self.copilotManager.get_inline_completion(
+                before, after, self._on_completion_ready
             )
-            self.copilotPanel.create_requested.connect(
-                lambda prompt, content_type: self.copilotManager.create_content(
-                    prompt, content_type, self._on_content_created
-                )
+        )
+        self.copilotPanel.edit_requested.connect(
+            lambda text, instruction: self.copilotManager.edit_text(
+                text, instruction, self._on_edit_ready
             )
-            self.copilotPanel.chat_requested.connect(self._on_chat_requested)
-            
-            # Connect copilot manager signals
-            self.copilotManager.completion_ready.connect(self._on_completion_ready)
-            self.copilotManager.chat_response.connect(self._on_chat_response)
-            self.copilotManager.status_changed.connect(
-                lambda status: self.copilotPanel.update_status(status)
+        )
+        self.copilotPanel.create_requested.connect(
+            lambda prompt, content_type: self.copilotManager.create_content(
+                prompt, content_type, self._on_content_created
             )
-            self.copilotManager.error_occurred.connect(self._on_copilot_error)
-            
-            self._copilot_signals_connected = True
+        )
+        self.copilotPanel.chat_requested.connect(self._on_chat_requested)
+        
+        # Connect copilot manager signals
+        self.copilotManager.completion_ready.connect(self._on_completion_ready)
+        self.copilotManager.chat_response.connect(self._on_chat_response)
+        self.copilotManager.status_changed.connect(
+            lambda status: self.copilotPanel.update_status(status)
+        )
+        self.copilotManager.error_occurred.connect(self._on_copilot_error)
+        
+        self._copilot_signals_connected = True
 
     def requestInlineCompletion(self):
         """请求行内补全"""
@@ -1369,15 +1375,18 @@ def requestInlineCompletion(self):
         if not hasattr(self, 'editor'):
             return
 
+        # Import constants from copilot_manager
+        from src.copilot.copilot_manager import MAX_CONTEXT_BEFORE, MAX_CONTEXT_AFTER
+        
         # 获取当前光标位置的上下文
         cursor = self.editor.editor.textCursor()
         text_before = self.editor.editor.toPlainText()[:cursor.position()]
         text_after = self.editor.editor.toPlainText()[cursor.position():]
 
-        # 请求补全
+        # 请求补全 - use constants for context window
         self.copilotManager.get_inline_completion(
-            text_before[-500:],  # 最多500字符上文
-            text_after[:100],    # 最多100字符下文
+            text_before[-MAX_CONTEXT_BEFORE:] if len(text_before) > MAX_CONTEXT_BEFORE else text_before,
+            text_after[:MAX_CONTEXT_AFTER] if len(text_after) > MAX_CONTEXT_AFTER else text_after,
             self._on_completion_ready
         )