-
Notifications
You must be signed in to change notification settings - Fork 125
fix(sandbox): ensure skills and attachments are ready #2587
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
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 |
|---|---|---|
|
|
@@ -11,8 +11,10 @@ | |
| import json | ||
| import logging | ||
| import os | ||
| import re | ||
| import time | ||
| from typing import Optional | ||
| from urllib.parse import urlsplit, urlunsplit | ||
|
|
||
| from langchain_core.callbacks import CallbackManagerForToolRun | ||
| from pydantic import BaseModel, Field | ||
|
|
@@ -21,6 +23,28 @@ | |
|
|
||
| # Default API base URL for attachment downloads | ||
| DEFAULT_API_BASE_URL = "http://backend:8000" | ||
| _ATTACHMENT_DOWNLOAD_PATH = re.compile( | ||
| r"^/api/attachments/(?P<attachment_id>\d+)/download/?$" | ||
| ) | ||
|
|
||
|
|
||
| def _build_download_url(attachment_url: str, api_base_url: str) -> str: | ||
| """Build a URL that accepts the task token available to sandbox tools.""" | ||
| relative_url = ( | ||
| attachment_url if attachment_url.startswith("/") else f"/{attachment_url}" | ||
| ) | ||
| parsed = urlsplit( | ||
| attachment_url | ||
| if attachment_url.startswith(("http://", "https://")) | ||
| else relative_url | ||
| ) | ||
| match = _ATTACHMENT_DOWNLOAD_PATH.fullmatch(parsed.path) | ||
| if not match: | ||
| raise ValueError("Only Wegent attachment download URLs are supported") | ||
|
|
||
| backend = urlsplit(api_base_url.rstrip("/")) | ||
| executor_path = f"/api/attachments/{match.group('attachment_id')}/executor-download" | ||
| return urlunsplit((backend.scheme, backend.netloc, executor_path, "", "")) | ||
|
|
||
|
|
||
| class SandboxDownloadAttachmentInput(BaseModel): | ||
|
|
@@ -61,7 +85,7 @@ class SandboxDownloadAttachmentTool(BaseSandboxTool): | |
| """Tool for downloading files from Wegent Backend to E2B sandbox. | ||
|
|
||
| This tool downloads files from Wegent's attachment storage to the | ||
| sandbox environment via the /api/attachments/{id}/download endpoint. | ||
| sandbox environment via the task-token attachment endpoint. | ||
| """ | ||
|
|
||
| name: str = "download_attachment" | ||
|
|
@@ -132,8 +156,8 @@ async def _arun( | |
| effective_timeout = timeout_seconds or self.default_download_timeout | ||
|
|
||
| logger.info( | ||
| f"[SandboxDownloadAttachmentTool] Downloading: {attachment_url} -> {save_path}, " | ||
| f"timeout={effective_timeout}s" | ||
| "[SandboxDownloadAttachmentTool] Downloading attachment: " | ||
| f"save_path={save_path}, timeout={effective_timeout}s" | ||
| ) | ||
|
|
||
| # Emit status update via WebSocket if available | ||
|
|
@@ -159,7 +183,7 @@ async def _arun( | |
|
|
||
| # Get or create sandbox | ||
| logger.info( | ||
| f"[SandboxDownloadAttachmentTool] Getting or creating sandbox..." | ||
| "[SandboxDownloadAttachmentTool] Getting or creating sandbox..." | ||
| ) | ||
| sandbox, error = await sandbox_manager.get_or_create_sandbox( | ||
| shell_type=self.default_shell_type, | ||
|
|
@@ -202,17 +226,10 @@ async def _arun( | |
| ) | ||
| api_base_url = api_base_url.rstrip("/") | ||
|
|
||
| # Build full download URL | ||
| # attachment_url can be relative (e.g., /api/attachments/123/download) or full URL | ||
| if attachment_url.startswith("http://") or attachment_url.startswith( | ||
| "https://" | ||
| ): | ||
| download_url = attachment_url | ||
| else: | ||
| # Ensure attachment_url starts with / | ||
| if not attachment_url.startswith("/"): | ||
| attachment_url = f"/{attachment_url}" | ||
| download_url = f"{api_base_url}{attachment_url}" | ||
| # The attachment block exposes the browser download URL. Translate | ||
| # that exact Wegent route to the executor route because sandbox tools | ||
| # authenticate with a task token rather than a browser login token. | ||
| download_url = _build_download_url(attachment_url, api_base_url) | ||
|
Comment on lines
+229
to
+232
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.
The tool argument is model-controlled, and this translation sends any numeric attachment ID to Useful? React with 👍 / 👎. |
||
|
|
||
| # Get auth token | ||
| auth_token = self.auth_token | ||
|
|
@@ -226,23 +243,27 @@ async def _arun( | |
| await self._emit_tool_status("failed", error_msg) | ||
| return result | ||
|
|
||
| # Build curl command to download file | ||
| # Keep credentials and user-provided paths out of the command string. | ||
| # E2B passes these values directly as process environment variables. | ||
| curl_cmd = ( | ||
| f"curl -s -f -L " | ||
| f'-H "Authorization: Bearer {auth_token}" ' | ||
| f'-o "{save_path}" ' | ||
| f'"{download_url}"' | ||
| "curl --silent --show-error --fail --location " | ||
| '--header "Authorization: Bearer $WEGENT_ATTACHMENT_TOKEN" ' | ||
| '--output "$WEGENT_ATTACHMENT_SAVE_PATH" ' | ||
| '"$WEGENT_ATTACHMENT_DOWNLOAD_URL"' | ||
| ) | ||
|
|
||
| logger.info( | ||
| f"[SandboxDownloadAttachmentTool] Executing download via curl from {download_url}" | ||
| ) | ||
| logger.info("[SandboxDownloadAttachmentTool] Executing attachment download") | ||
|
|
||
| # Execute curl command | ||
| result_obj = await sandbox.commands.run( | ||
| cmd=curl_cmd, | ||
| cwd="/home/user", | ||
| timeout=effective_timeout, | ||
| envs={ | ||
| "WEGENT_ATTACHMENT_TOKEN": auth_token, | ||
| "WEGENT_ATTACHMENT_SAVE_PATH": save_path, | ||
| "WEGENT_ATTACHMENT_DOWNLOAD_URL": download_url, | ||
| }, | ||
| ) | ||
|
|
||
| execution_time = time.time() - start_time | ||
|
|
||
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.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use all required Kind identifiers during the update.
Lines 473-479 introduce an update path that calls
update_skillwith onlyskill_idanduser_id. The provided implementation inbackend/app/services/adapters/skill_kinds.py:677-778then queries the Kind withoutnamespaceorname.Extend the update service contract and its Kind query to require and match
namespace,name, anduser_id. Add coverage that verifies these identity predicates.As per coding guidelines: “A Kind resource must always be queried using all three identifiers:
namespace,name, anduser_id.”🤖 Prompt for AI Agents
Source: Coding guidelines