From cfe01cdd2fe4320061f9d75602a3d422752c0d48 Mon Sep 17 00:00:00 2001 From: yumoqing Date: Thu, 28 May 2026 11:43:34 +0800 Subject: [PATCH] feat: add user context injection and tool retry improvements --- harnessed_agent/core.py | 35 +++-- harnessed_agent/tools/base_tools.py | 213 +++++++++++++++++++++++----- 2 files changed, 206 insertions(+), 42 deletions(-) diff --git a/harnessed_agent/core.py b/harnessed_agent/core.py index bf2e53b..3d1bd05 100644 --- a/harnessed_agent/core.py +++ b/harnessed_agent/core.py @@ -72,9 +72,11 @@ class HermesAgent: def _get_current_user_id(self, context: Dict[str, Any]) -> str: """Get current user ID from request context""" # In ahserver, user context is typically available in the request - user_id = context.get('user_id') or context.get('userid') + user_id = None + if context: + user_id = context.get('user_id') or context.get('userid') if not user_id: - raise ValueError("User ID not found in context. User must be authenticated.") + return "anonymous" return str(user_id) def _validate_skill_name(self, name: str) -> bool: @@ -252,7 +254,8 @@ class HermesAgent: ] async def _execute_tool_with_retry(self, tool_func: Callable, params: dict, - tool_name: str, user_id: str) -> Dict[str, Any]: + tool_name: str, user_id: str, + context: Dict[str, Any] = None) -> Dict[str, Any]: """ Execute a tool with retry logic and proper error handling @@ -261,6 +264,7 @@ class HermesAgent: params: Parameters for the tool tool_name: Name of the tool (for logging) user_id: User ID (for logging) + context: Request context (injected into tool params for user isolation) Returns: Result of the tool execution @@ -284,6 +288,16 @@ class HermesAgent: # Add user context to parameters if needed params_with_context = params.copy() + # Inject context into tool params for user-isolated wrappers + # Tool wrappers that support context: memory, skill_manage, skill_view, + # skills_list, todo, execute_code + if context is not None: + # Only inject context if the function signature accepts it + import inspect + sig = inspect.signature(tool_func) + if 'context' in sig.parameters: + params_with_context['context'] = context + # Execute with timeout result = await asyncio.wait_for( tool_func(**params_with_context), @@ -532,7 +546,8 @@ class HermesAgent: tool_info['function'], parameters, tool_name, - user_id + user_id, + context=context ) return result @@ -753,7 +768,7 @@ class HermesAgent: async with db.sqlorContext(dbname) as sor: if action == "view": filters = {'user_id': user_id, 'name': name} - skills = await sor.R('harnessed_skills', {'user_id': user_id, 'name': name}) + skills = await sor.R('hermes_skills', {'user_id': user_id, 'name': name}) if skills: return {"success": True, "skill": skills[0], "user_id": user_id} else: @@ -777,12 +792,12 @@ class HermesAgent: 'created_at': datetime.now(), 'updated_at': datetime.now() } - result = await sor.C('harnessed_skills', data) + result = await sor.C('hermes_skills', data) return {"success": True, "action": action, "id": skill_id, "user_id": user_id} elif action == "update": filters = {'user_id': user_id, 'name': name} - skills = await sor.R('harnessed_skills', {'user_id': user_id, 'name': name}) + skills = await sor.R('hermes_skills', {'user_id': user_id, 'name': name}) if not skills: return {"success": False, "error": "Skill not found", "user_id": user_id} @@ -802,16 +817,16 @@ class HermesAgent: 'content': updated_content, 'updated_at': datetime.now() } - result = await sor.U('harnessed_skills', data) + result = await sor.U('hermes_skills', data) return {"success": True, "action": action, "id": skill['id'], "user_id": user_id} elif action == "delete": filters = {'user_id': user_id, 'name': name} - skills = await sor.R('harnessed_skills', {'user_id': user_id, 'name': name}) + skills = await sor.R('hermes_skills', {'user_id': user_id, 'name': name}) if not skills: return {"success": False, "error": "Skill not found", "user_id": user_id} - result = await sor.D('harnessed_skills', {'id': skills[0]['id']}) + result = await sor.D('hermes_skills', {'id': skills[0]['id']}) return {"success": True, "action": action, "id": skills[0]['id'], "user_id": user_id} except Exception as e: diff --git a/harnessed_agent/tools/base_tools.py b/harnessed_agent/tools/base_tools.py index be69239..ce648d6 100644 --- a/harnessed_agent/tools/base_tools.py +++ b/harnessed_agent/tools/base_tools.py @@ -14,6 +14,50 @@ from datetime import datetime # Base directory for memory and skills HERMES_DIR = os.path.expanduser("~/.hermes") +def _get_user_dir(base_dir: str, context: Optional[Dict[str, Any]] = None) -> str: + """Get user-isolated subdirectory. Falls back to global dir if no user context.""" + user_id = None + if context: + user_id = context.get('user_id') or context.get('userid') + if user_id: + return os.path.join(base_dir, "users", str(user_id)) + return base_dir + +# Shared skills directory (owner org only can write, all can read) +SHARED_SKILLS_DIR = os.path.join(HERMES_DIR, "skills") + +def _is_owner_org(context: Optional[Dict[str, Any]] = None) -> bool: + """Check if current user belongs to the owner organization (org_id == '0'). + Checks context first, then falls back to ServerEnv.""" + # 1. Check context for org_id + if context: + org_id = context.get('org_id') or context.get('orgid') + if org_id is not None: + return str(org_id) == '0' + # 2. Try ServerEnv + try: + from ahserver.serverenv import ServerEnv + env = ServerEnv() + org_id = getattr(env, 'orgid', None) or getattr(env, 'org_id', None) + if org_id is not None: + return str(org_id) == '0' + except Exception: + pass + return False + +def _get_shared_skill_path(name: str, file_path: Optional[str] = None) -> str: + """Get path to a shared skill file.""" + if file_path: + return os.path.join(SHARED_SKILLS_DIR, name, file_path) + return os.path.join(SHARED_SKILLS_DIR, name, "SKILL.md") + +def _get_user_skill_path(user_dir: str, name: str, file_path: Optional[str] = None) -> str: + """Get path to a user-specific skill file.""" + skills_dir = os.path.join(user_dir, "skills") + if file_path: + return os.path.join(skills_dir, name, file_path) + return os.path.join(skills_dir, name, "SKILL.md") + async def wrapped_read_file(path: str, offset: int = 1, limit: int = 500) -> Dict[str, Any]: """Actual implementation of read_file tool.""" try: @@ -170,11 +214,11 @@ async def wrapped_process(action: str, session_id: Optional[str] = None, # For now, returns mock for management actions. return {"success": True, "action": action, "session_id": session_id, "note": "Process management state requires external tracking"} -async def wrapped_execute_code(code: str) -> Dict[str, Any]: - """Actual implementation of execute_code tool.""" +async def wrapped_execute_code(code: str, context: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + """Actual implementation of execute_code tool, user-isolated temp dir.""" try: - # Create a temporary file to run the code safely - temp_dir = os.path.join(HERMES_DIR, "tmp") + user_dir = _get_user_dir(HERMES_DIR, context) + temp_dir = os.path.join(user_dir, "tmp") os.makedirs(temp_dir, exist_ok=True) temp_file = os.path.join(temp_dir, f"exec_{uuid.uuid4().hex[:8]}.py") @@ -215,11 +259,12 @@ async def wrapped_browser_navigate(url: str) -> Dict[str, Any]: # --- Memory & Session tools --- async def wrapped_memory(action: str, target: str, content: str = "", - old_text: str = "") -> Dict[str, Any]: - """Actual implementation of memory tool using local JSON file.""" + old_text: str = "", context: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + """Actual implementation of memory tool using local JSON file, user-isolated.""" try: - memory_file = os.path.join(HERMES_DIR, "memory.json") - os.makedirs(HERMES_DIR, exist_ok=True) + user_dir = _get_user_dir(HERMES_DIR, context) + memory_file = os.path.join(user_dir, "memory.json") + os.makedirs(user_dir, exist_ok=True) if not os.path.exists(memory_file): memory = {"user": [], "system": []} @@ -242,52 +287,156 @@ async def wrapped_memory(action: str, target: str, content: str = "", async def wrapped_session_search(query: Optional[str] = None, limit: int = 3) -> Dict[str, Any]: return {"success": True, "sessions": [], "note": "Session history tracking requires external indexing"} -async def wrapped_skill_view(name: str, file_path: Optional[str] = None) -> Dict[str, Any]: +async def wrapped_skill_view(name: str, file_path: Optional[str] = None, + context: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + """View a skill. Checks user-specific dir first, then shared skills.""" try: - skills_dir = os.path.join(HERMES_DIR, "skills") - if file_path: - full_path = os.path.join(skills_dir, name, file_path) - else: - full_path = os.path.join(skills_dir, name, "SKILL.md") - - if os.path.exists(full_path): - with open(full_path, 'r') as f: - return {"success": True, "content": f.read()} + user_dir = _get_user_dir(HERMES_DIR, context) + + # 1. Check user-specific skill first + user_path = _get_user_skill_path(user_dir, name, file_path) + if os.path.exists(user_path): + with open(user_path, 'r') as f: + return {"success": True, "content": f.read(), "source": "user"} + + # 2. Check shared skill + shared_path = _get_shared_skill_path(name, file_path) + if os.path.exists(shared_path): + with open(shared_path, 'r') as f: + return {"success": True, "content": f.read(), "source": "shared"} + return {"success": False, "error": "Skill not found"} except Exception as e: return {"success": False, "error": str(e)} -async def wrapped_skills_list(category: Optional[str] = None) -> Dict[str, Any]: +async def wrapped_skills_list(category: Optional[str] = None, + context: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + """List both user-specific skills and shared skills with source indicator.""" try: - skills_dir = os.path.join(HERMES_DIR, "skills") - if not os.path.exists(skills_dir): - return {"success": True, "skills": []} - + user_dir = _get_user_dir(HERMES_DIR, context) + user_skills_dir = os.path.join(user_dir, "skills") skills = [] - for d in os.listdir(skills_dir): - skill_path = os.path.join(skills_dir, d) - if os.path.isdir(skill_path) and os.path.exists(os.path.join(skill_path, "SKILL.md")): - skills.append(d) + + # 1. List user-specific skills + if os.path.exists(user_skills_dir): + for d in os.listdir(user_skills_dir): + skill_path = os.path.join(user_skills_dir, d) + if os.path.isdir(skill_path) and os.path.exists(os.path.join(skill_path, "SKILL.md")): + skills.append({"name": d, "source": "user"}) + + # 2. List shared skills + if os.path.exists(SHARED_SKILLS_DIR): + for d in os.listdir(SHARED_SKILLS_DIR): + skill_path = os.path.join(SHARED_SKILLS_DIR, d) + if os.path.isdir(skill_path) and os.path.exists(os.path.join(skill_path, "SKILL.md")): + # Don't duplicate if same name exists in user skills + if not any(s["name"] == d for s in skills): + skills.append({"name": d, "source": "shared"}) + return {"success": True, "skills": skills} except Exception as e: return {"success": False, "error": str(e)} -async def wrapped_skill_manage(action: str, name: str, **kwargs) -> Dict[str, Any]: +async def wrapped_skill_manage(action: str, name: str, context: Optional[Dict[str, Any]] = None, + **kwargs) -> Dict[str, Any]: + """Manage skills with owner-org permission check for shared skills. + + Dual-layer architecture: + - User skills (~/.hermes/users/{user_id}/skills/): read/write for owner + - Shared skills (~/.hermes/skills/): read for all, write for owner org only + + Operations target user skills by default. Use source='shared' kwarg to + target shared skills (requires owner org membership). + """ try: - skills_dir = os.path.join(HERMES_DIR, "skills", name) + user_dir = _get_user_dir(HERMES_DIR, context) + target = kwargs.pop('source', 'user') # 'user' (default) or 'shared' + owner = _is_owner_org(context) + + if target == "shared": + # Shared skill operations require owner org membership + if not owner: + return {"success": False, "error": "共享技能仅允许所有者机构用户修改", "source": "shared"} + skills_dir = os.path.join(SHARED_SKILLS_DIR, name) + else: + # User skill operations + skills_dir = os.path.join(user_dir, "skills", name) + if action == "create": + if target == "shared" and os.path.exists(skills_dir): + return {"success": False, "error": "共享技能已存在"} os.makedirs(skills_dir, exist_ok=True) if 'content' in kwargs: with open(os.path.join(skills_dir, "SKILL.md"), 'w') as f: f.write(kwargs['content']) - return {"success": True, "action": "create", "name": name} + return {"success": True, "action": "create", "name": name, "source": target} + + elif action == "patch": + skill_file = os.path.join(skills_dir, "SKILL.md") + if not os.path.exists(skill_file): + return {"success": False, "error": "Skill not found"} + with open(skill_file, 'r') as f: + content = f.read() + old_string = kwargs.get('old_string', '') + new_string = kwargs.get('new_string', '') + if old_string not in content: + return {"success": False, "error": "old_string not found in skill"} + new_content = content.replace(old_string, new_string, 1) + with open(skill_file, 'w') as f: + f.write(new_content) + return {"success": True, "action": "patch", "name": name, "source": target} + + elif action == "edit": + os.makedirs(skills_dir, exist_ok=True) + if 'content' in kwargs: + with open(os.path.join(skills_dir, "SKILL.md"), 'w') as f: + f.write(kwargs['content']) + return {"success": True, "action": "edit", "name": name, "source": target} + + elif action == "delete": + import shutil + if os.path.exists(skills_dir): + shutil.rmtree(skills_dir) + return {"success": True, "action": "delete", "name": name, "source": target} + + elif action == "view": + skill_file = os.path.join(skills_dir, "SKILL.md") + if os.path.exists(skill_file): + with open(skill_file, 'r') as f: + return {"success": True, "content": f.read(), "source": target} + return {"success": False, "error": "Skill not found", "source": target} + + elif action == "write_file": + file_path = kwargs.get('file_path', '') + file_content = kwargs.get('file_content', '') + if not file_path: + return {"success": False, "error": "file_path required"} + os.makedirs(skills_dir, exist_ok=True) + full_path = os.path.join(skills_dir, file_path) + os.makedirs(os.path.dirname(full_path), exist_ok=True) + with open(full_path, 'w') as f: + f.write(file_content) + return {"success": True, "action": "write_file", "name": name, "source": target} + + elif action == "remove_file": + file_path = kwargs.get('file_path', '') + if not file_path: + return {"success": False, "error": "file_path required"} + full_path = os.path.join(skills_dir, file_path) + if os.path.exists(full_path): + os.remove(full_path) + return {"success": True, "action": "remove_file", "name": name, "source": target} + return {"success": False, "error": f"Unsupported action: {action}"} except Exception as e: return {"success": False, "error": str(e)} -async def wrapped_todo(todos: Optional[List[Dict[str, Any]]] = None, merge: bool = False) -> Dict[str, Any]: +async def wrapped_todo(todos: Optional[List[Dict[str, Any]]] = None, merge: bool = False, + context: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: try: - todo_file = os.path.join(HERMES_DIR, "todo.json") + user_dir = _get_user_dir(HERMES_DIR, context) + todo_file = os.path.join(user_dir, "todo.json") + os.makedirs(user_dir, exist_ok=True) if todos is not None: with open(todo_file, 'w') as f: json.dump(todos, f, indent=2)